netdev - Re: [PATCH bpf-next v3 10/17] bpf: Prevent KASAN false positive with bpf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date: Mon, 18 Sep 2023 15:20:31 +0200
From: Matthieu Baerts <matthieu.baerts@...sares.net>
To: Kumar Kartikeya Dwivedi <memxor@...il.com>, bpf@...r.kernel.org
Cc: Andrey Ryabinin <ryabinin.a.a@...il.com>,
 Alexander Potapenko <glider@...gle.com>,
 Andrey Konovalov <andreyknvl@...il.com>, Dmitry Vyukov <dvyukov@...gle.com>,
 Vincenzo Frascino <vincenzo.frascino@....com>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Yonghong Song <yonghong.song@...ux.dev>,
 David Vernet <void@...ifault.com>, Puranjay Mohan <puranjay12@...il.com>,
 netdev <netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>,
 Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
 MPTCP Upstream <mptcp@...ts.linux.dev>
Subject: Re: [PATCH bpf-next v3 10/17] bpf: Prevent KASAN false positive with
 bpf_throw

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