bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Magnus Karlsson <magnus.karlsson@gmail.com>
To: magnus.karlsson@intel.com, bjorn.topel@intel.com, ast@kernel.org,
	daniel@iogearbox.net, netdev@vger.kernel.org,
	jonathan.lemon@gmail.com
Cc: bpf@vger.kernel.org
Subject: [PATCH bpf-next] xsk: fix possible crash in socket_release when out-of-memory
Date: Sat, 26 Sep 2020 11:26:13 +0200	[thread overview]
Message-ID: <1601112373-10595-1-git-send-email-magnus.karlsson@gmail.com> (raw)

From: Magnus Karlsson <magnus.karlsson@intel.com>

Fix possible crash in socket_release when an out-of-memory error has
occurred in the bind call. If a socket using the XDP_SHARED_UMEM flag
encountered an error in xp_create_and_assign_umem, the bind code
jumped to the exit routine but erroneously forgot to set the err value
before jumping. This meant that the exit routine thought the setup
went well and set the state of the socket to XSK_BOUND. The xsk socket
release code will then, at application exit, think that this is a
properly setup socket, when it is not, leading to a crash when all
fields in the socket have in fact not been initialized properly. Fix
this by setting the err variable in xsk_bind so that the socket is not
set to XSK_BOUND which leads to the clean-up in xsk_release not being
triggered.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Reported-by: syzbot+ddc7b4944bc61da19b81@syzkaller.appspotmail.com
Fixes: 1c1efc2af158 ("xsk: Create and free buffer pool independently from umem")
---
I have not been able to reproduce this issue using the syzkaller
config and reproducer, so I cannot guarantee it fixes it. But this bug
is real and it is triggered by an out-of-memory in
xp_create_and_assign_umem, just like syzcaller injects, and would lead
to the same crash in dev_hold in xsk_release.
---
 net/xdp/xsk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 3895697..ba4dfb1 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -703,6 +703,7 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
 			xs->pool = xp_create_and_assign_umem(xs,
 							     umem_xs->umem);
 			if (!xs->pool) {
+				err = -ENOMEM;
 				sockfd_put(sock);
 				goto out_unlock;
 			}
-- 
2.7.4


             reply	other threads:[~2020-09-26  9:26 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-26  9:26 Magnus Karlsson [this message]
2020-09-28 19:25 ` [PATCH bpf-next] xsk: fix possible crash in socket_release when out-of-memory Daniel Borkmann
2020-09-29 12:20 ` patchwork-bot+bpf

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=1601112373-10595-1-git-send-email-magnus.karlsson@gmail.com \
    --to=magnus.karlsson@gmail.com \
    --cc=ast@kernel.org \
    --cc=bjorn.topel@intel.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jonathan.lemon@gmail.com \
    --cc=magnus.karlsson@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).