Andrii Nakryiko wrote: > On Fri, Oct 30, 2020 at 5:08 AM Kevin Sheldrake > <Kevin.Sheldrake@xxxxxxxxxxxxx> wrote: > > > > As discussed, bpf_perf_event_output() takes a u64 for the sample size parameter but the perf ring buffer uses a u16 internally. This results in overlapping samples where the total sample size (including header/padding) exceeds 64K, and prevents samples from being submitted when the total sample size == 64K. [...] > >Also I don't know what the size reduction of -24 relates to (it doesn't match any header struct I've found) but it was found through experimentation. > > So -24 should have been a clue that something fishy is going on. Look > at perf_prepare_sample() in kernel/events/core.c. header->size (which > is u16) contains the entire size of the data in the perf event. This > includes raw data that you send with bpf_perf_event_output(), but it > can also have tons of other stuff (e.g., call stacks, LBR data, etc). > What gets added to the perf sample depends on how the perf event was > configured in the first place. And it happens automatically on each > perf event output. > > So, all that means that there could be no reliable static check in the > verifier which would prevent the corruption. It has to be checked by > perf_prepare_sample() in runtime based on the actual size of the > sample. We can do an extra check in verifier, but I wouldn't bother > because it's never going to be 100% correct. Please don't add the check in the verifier if its not 100% correct. I think that will confuse readers and make it appear "safe" when it is not. Even if you add a big warning comment there it will make the error case harder to hit. So lets just solve it as Andrii notes. My $.02 Thanks.
