All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Sitnicki <jakub@cloudflare.com>
To: John Fastabend <john.fastabend@gmail.com>
Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
	bpf@vger.kernel.org, joamaki@gmail.com
Subject: Re: [PATCH bpf-next] bpf, sockmap: fix double bpf_prog_put on error case in map_link
Date: Sat, 08 Jan 2022 15:10:03 +0100	[thread overview]
Message-ID: <874k6epbd0.fsf@cloudflare.com> (raw)
In-Reply-To: <20220104214645.290900-1-john.fastabend@gmail.com>

On Tue, Jan 04, 2022 at 10:46 PM CET, John Fastabend wrote:
> sock_map_link() is called to update a sockmap entry with a sk. But, if the
> sock_map_init_proto() call fails then we return an error to the map_update
> op against the sockmap. In the error path though we need to cleanup psock
> and dec the refcnt on any programs associated with the map, because we
> refcnt them early in the update process to ensure they are pinned for the
> psock. (This avoids a race where user deletes programs while also updating
> the map with new socks.)
>
> In current code we do the prog refcnt dec explicitely by calling
> bpf_prog_put() when the program was found in the map. But, after commit
> '38207a5e81230' in this error path we've already done the prog to psock
> assignment so the programs have a reference from the psock as well. This
> then causes the psock tear down logic, invoked by sk_psock_put() in the
> error path, to similarly call bpf_prog_put on the programs there.
>
> To be explicit this logic does the prog->psock assignemnt
>
>   if (msg_*)
>     psock_set_prog(...)
>
> Then the error path under the out_progs label does a similar check and dec
> with,
>
>   if (msg_*)
>      bpf_prog_put(...)
>
> And the teardown logic sk_psock_put() does,
>
>   psock_set_prog(msg_*, NULL)
>
> triggering another bpf_prog_put(...). Then KASAN gives us this splat, found
> by syzbot because we've created an inbalance between bpf_prog_inc and
> bpf_prog_put calling put twice on the program.
>
> BUG: KASAN: vmalloc-out-of-bounds in __bpf_prog_put kernel/bpf/syscall.c:1812 [inline]
> BUG: KASAN: vmalloc-out-of-bounds in __bpf_prog_put kernel/bpf/syscall.c:1812 [inline] kernel/bpf/syscall.c:1829
> BUG: KASAN: vmalloc-out-of-bounds in bpf_prog_put+0x8c/0x4f0 kernel/bpf/syscall.c:1829 kernel/bpf/syscall.c:1829
> Read of size 8 at addr ffffc90000e76038 by task syz-executor020/3641
>
> To fix clean up error path so it doesn't try to do the bpf_prog_put in the
> error path once progs are assigned then it relies on the normal psock
> tear down logic to do complete cleanup.
>
> For completness we also cover the case whereh sk_psock_init_strp() fails,
> but this is not expected because it indicates an incorrect socket type
> and should be caught earlier.
>
> Reported-by: syzbot+bb73e71cf4b8fd376a4f@syzkaller.appspotmail.com
> Fixes: 38207a5e8123 ("bpf, sockmap: Attach map progs to psock early for feature probes")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---

FWIW, late :thumbup:

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>

      parent reply	other threads:[~2022-01-08 14:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-04 21:46 [PATCH bpf-next] bpf, sockmap: fix double bpf_prog_put on error case in map_link John Fastabend
2022-01-05 19:50 ` patchwork-bot+netdevbpf
2022-01-08 14:10 ` Jakub Sitnicki [this message]

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=874k6epbd0.fsf@cloudflare.com \
    --to=jakub@cloudflare.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=joamaki@gmail.com \
    --cc=john.fastabend@gmail.com \
    --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.