On Mon, 18 Sept 2023 at 15:20, Matthieu Baerts <matthieu.baerts@xxxxxxxxxxxx> wrote: > > Hi Kumar, > > (+ netdev in Cc as this patch is now in net-next tree as well ; same for > mptcp-next) > > > On 13/09/2023 01:32, Kumar Kartikeya Dwivedi wrote: > > The KASAN stack instrumentation when CONFIG_KASAN_STACK is true poisons > > the stack of a function when it is entered and unpoisons it when > > leaving. However, in the case of bpf_throw, we will never return as we > > switch our stack frame to the BPF exception callback. Later, this > > discrepancy will lead to confusing KASAN splats when kernel resumes > > execution on return from the BPF program. > > > > Fix this by unpoisoning everything below the stack pointer of the BPF > > program, which should cover the range that would not be unpoisoned. An > > example splat is below: > > Thank you for your patch! > > (...) > > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > > index 78e8f4de6750..2c8e1ee97b71 100644 > > --- a/kernel/bpf/helpers.c > > +++ b/kernel/bpf/helpers.c > > @@ -22,6 +22,7 @@ > > #include <linux/security.h> > > #include <linux/btf_ids.h> > > #include <linux/bpf_mem_alloc.h> > > +#include <linux/kasan.h> > > > > #include "../../lib/kstrtox.h" > > > > @@ -2483,6 +2484,11 @@ __bpf_kfunc void bpf_throw(u64 cookie) > > WARN_ON_ONCE(!ctx.aux->exception_boundary); > > WARN_ON_ONCE(!ctx.bp); > > WARN_ON_ONCE(!ctx.cnt); > > + /* Prevent KASAN false positives for CONFIG_KASAN_STACK by unpoisoning > > + * deeper stack depths than ctx.sp as we do not return from bpf_throw, > > + * which skips compiler generated instrumentation to do the same. > > + */ > > + kasan_unpoison_task_stack_below((void *)ctx.sp); > > Our CI validating MPTCP tree has just reported the following error when > building the kernel for a 32-bit architecture: > > kernel/bpf/helpers.c: In function 'bpf_throw': > kernel/bpf/helpers.c:2491:41: error: cast to pointer from integer of > different size [-Werror=int-to-pointer-cast] > 2491 | kasan_unpoison_task_stack_below((void *)ctx.sp); > | ^ > cc1: all warnings being treated as errors > > Source: > https://github.com/multipath-tcp/mptcp_net-next/actions/runs/6221288400/job/16882945173 > > > It looks like this issue has been introduced by your patch. Are you > already looking at a fix? > Yes, my patch is responsible. So pointers here are 32-bits, while ctx.sp is 64-bit, hence it is complaining. I think long is supposed to match pointer width on all targets Linux supports, so doing this should fix it. (void*)(long)ctx.sp I will send a fix for this soon. Thanks > > ctx.aux->bpf_exception_cb(cookie, ctx.sp, ctx.bp); > > } > > > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net
