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?
> ctx.aux->bpf_exception_cb(cookie, ctx.sp, ctx.bp);
> }
>
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
