On 1/23/20 2:30 PM, Daniel Xu wrote: > On Thu Jan 23, 2020 at 11:23 PM, Daniel Borkmann wrote: > [...] >> >> Yes, so we've been following this practice for all the BPF helpers no >> matter >> which program type. Though for tracing it may be up to debate whether it >> makes >> still sense given there's nothing to be leaked here since you can read >> this data >> anyway via probe read if you'd wanted to. So we might as well get rid of >> the >> clearing for all tracing helpers. > > Right, that makes sense. Do you want me to leave it in for this patchset > and then remove all of them in a followup patchset? > I don't think we can remove that for existing tracing helpers (e.g., bpf_probe_read). There are applications that explicitly expect destination memory to be zeroed out on failure. It's a BPF world's memset(0). I also wonder if BPF verifier has any extra assumptions for ARG_PTR_TO_UNINIT_MEM w.r.t. it being initialized after helper call (e.g., for liveness tracking). >> >> Different question related to your set. It looks like br_stack is only >> available >> on x86, is that correct? For other archs this will always bail out on >> !br_stack >> test. Perhaps we should document this fact so users are not surprised >> why their >> prog using this helper is not working on !x86. Wdyt? > > I think perf_event_open() should fail on !x86 if a user tries to configure > it with branch stack collection. So there would not be the opportunity for > the bpf prog to be attached and run. I haven't tested this, though. I'll > look through the code / install a VM and test it. > > [...] > > Thanks, > Daniel >
