From: Jakub Sitnicki <jakub@cloudflare.com>
To: John Fastabend <john.fastabend@gmail.com>
Cc: bpf@vger.kernel.org, edumazet@google.com, martin.lau@kernel.org,
netdev@vger.kernel.org, kuniyu@amazon.com
Subject: Re: [PATCH bpf v2 1/2] bpf: syzkaller found null ptr deref in unix_bpf proto add
Date: Wed, 06 Dec 2023 10:47:42 +0100 [thread overview]
Message-ID: <87msunslt2.fsf@cloudflare.com> (raw)
In-Reply-To: <656e4758675b9_1bd6e2086f@john.notmuch>
On Mon, Dec 04, 2023 at 01:40 PM -08, John Fastabend wrote:
> Kuniyuki Iwashima wrote:
>> From: John Fastabend <john.fastabend@gmail.com>
>> Date: Fri, 1 Dec 2023 10:01:38 -0800
>> > I added logic to track the sock pair for stream_unix sockets so that we
>> > ensure lifetime of the sock matches the time a sockmap could reference
>> > the sock (see fixes tag). I forgot though that we allow af_unix unconnected
>> > sockets into a sock{map|hash} map.
>> >
>> > This is problematic because previous fixed expected sk_pair() to exist
>> > and did not NULL check it. Because unconnected sockets have a NULL
>> > sk_pair this resulted in the NULL ptr dereference found by syzkaller.
>> >
>> > BUG: KASAN: null-ptr-deref in unix_stream_bpf_update_proto+0x72/0x430
>> > net/unix/unix_bpf.c:171
>> > Write of size 4 at addr 0000000000000080 by task syz-executor360/5073
>> > Call Trace:
>> > <TASK>
>> > ...
>> > sock_hold include/net/sock.h:777 [inline]
>> > unix_stream_bpf_update_proto+0x72/0x430 net/unix/unix_bpf.c:171
>> > sock_map_init_proto net/core/sock_map.c:190 [inline]
>> > sock_map_link+0xb87/0x1100 net/core/sock_map.c:294
>> > sock_map_update_common+0xf6/0x870 net/core/sock_map.c:483
>> > sock_map_update_elem_sys+0x5b6/0x640 net/core/sock_map.c:577
>> > bpf_map_update_value+0x3af/0x820 kernel/bpf/syscall.c:167
>> >
>> > We considered just checking for the null ptr and skipping taking a ref
>> > on the NULL peer sock. But, if the socket is then connected() after
>> > being added to the sockmap we can cause the original issue again. So
>> > instead this patch blocks adding af_unix sockets that are not in the
>> > ESTABLISHED state.
>>
>> I'm not sure if someone has the unconnected stream socket use case
>> though, can't we call additional sock_hold() in connect() by checking
>> sk_prot under sk_callback_lock ?
>
> Could be done I guess yes. I'm not sure the utility of it though. I
> thought above patch was the simplest solution and didn't require touching
> main af_unix code. I don't actually use the sockmap with af_unix
> sockets anywhere so maybe someone who is using this can comment if
> unconnected is needed?
>
> From rcu and locking side looks like holding sk_callback_lock would
> be sufficient. I was thinking it would require a rcu grace period
> or something but seems not.
I'd revist the option of grabbing an skpair ref in unix_stream_sendmsg.
next prev parent reply other threads:[~2023-12-06 9:49 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-01 18:01 [PATCH bpf v2 0/2] bpf fix for unconnect af_unix socket John Fastabend
2023-12-01 18:01 ` [PATCH bpf v2 1/2] bpf: syzkaller found null ptr deref in unix_bpf proto add John Fastabend
2023-12-01 21:14 ` Kuniyuki Iwashima
2023-12-04 21:40 ` John Fastabend
2023-12-04 22:37 ` Kuniyuki Iwashima
2023-12-06 9:47 ` Jakub Sitnicki [this message]
2023-12-08 4:19 ` Cong Wang
2023-12-11 14:56 ` Daniel Borkmann
2023-12-13 23:23 ` [External] " Amery Hung
2023-12-01 18:01 ` [PATCH bpf v2 2/2] bpf: sockmap, test for unconnected af_unix sock John Fastabend
2023-12-12 10:09 ` [PATCH bpf v2 0/2] bpf fix for unconnect af_unix socket Jakub Sitnicki
2023-12-14 1:40 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87msunslt2.fsf@cloudflare.com \
--to=jakub@cloudflare.com \
--cc=bpf@vger.kernel.org \
--cc=edumazet@google.com \
--cc=john.fastabend@gmail.com \
--cc=kuniyu@amazon.com \
--cc=martin.lau@kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.