All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	syzbot+709412e651e55ed96498@syzkaller.appspotmail.com,
	syzbot+54f39d6ab58f39720a55@syzkaller.appspotmail.com,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Connor OBrien <connoro@google.com>
Subject: [PATCH 4.14 08/45] bpf: fix panic due to oob in bpf_prog_test_run_skb
Date: Mon, 20 Dec 2021 15:34:03 +0100	[thread overview]
Message-ID: <20211220143022.544815365@linuxfoundation.org> (raw)
In-Reply-To: <20211220143022.266532675@linuxfoundation.org>

From: Daniel Borkmann <daniel@iogearbox.net>

commit 6e6fddc78323533be570873abb728b7e0ba7e024 upstream.

sykzaller triggered several panics similar to the below:

  [...]
  [  248.851531] BUG: KASAN: use-after-free in _copy_to_user+0x5c/0x90
  [  248.857656] Read of size 985 at addr ffff8808017ffff2 by task a.out/1425
  [...]
  [  248.865902] CPU: 1 PID: 1425 Comm: a.out Not tainted 4.18.0-rc4+ #13
  [  248.865903] Hardware name: Supermicro SYS-5039MS-H12TRF/X11SSE-F, BIOS 2.1a 03/08/2018
  [  248.865905] Call Trace:
  [  248.865910]  dump_stack+0xd6/0x185
  [  248.865911]  ? show_regs_print_info+0xb/0xb
  [  248.865913]  ? printk+0x9c/0xc3
  [  248.865915]  ? kmsg_dump_rewind_nolock+0xe4/0xe4
  [  248.865919]  print_address_description+0x6f/0x270
  [  248.865920]  kasan_report+0x25b/0x380
  [  248.865922]  ? _copy_to_user+0x5c/0x90
  [  248.865924]  check_memory_region+0x137/0x190
  [  248.865925]  kasan_check_read+0x11/0x20
  [  248.865927]  _copy_to_user+0x5c/0x90
  [  248.865930]  bpf_test_finish.isra.8+0x4f/0xc0
  [  248.865932]  bpf_prog_test_run_skb+0x6a0/0xba0
  [...]

After scrubbing the BPF prog a bit from the noise, turns out it called
bpf_skb_change_head() for the lwt_xmit prog with headroom of 2. Nothing
wrong in that, however, this was run with repeat >> 0 in bpf_prog_test_run_skb()
and the same skb thus keeps changing until the pskb_expand_head() called
from skb_cow() keeps bailing out in atomic alloc context with -ENOMEM.
So upon return we'll basically have 0 headroom left yet blindly do the
__skb_push() of 14 bytes and keep copying data from there in bpf_test_finish()
out of bounds. Fix to check if we have enough headroom and if pskb_expand_head()
fails, bail out with error.

Another bug independent of this fix (but related in triggering above) is
that BPF_PROG_TEST_RUN should be reworked to reset the skb/xdp buffer to
it's original state from input as otherwise repeating the same test in a
loop won't work for benchmarking when underlying input buffer is getting
changed by the prog each time and reused for the next run leading to
unexpected results.

Fixes: 1cf1cae963c2 ("bpf: introduce BPF_PROG_TEST_RUN command")
Reported-by: syzbot+709412e651e55ed96498@syzkaller.appspotmail.com
Reported-by: syzbot+54f39d6ab58f39720a55@syzkaller.appspotmail.com
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
[connoro: drop test_verifier.c changes not applicable to 4.14]
Signed-off-by: Connor O'Brien <connoro@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/bpf/test_run.c                          |   17 ++++++++++++++---
 tools/testing/selftests/bpf/test_verifier.c |   18 ++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -96,6 +96,7 @@ int bpf_prog_test_run_skb(struct bpf_pro
 	u32 size = kattr->test.data_size_in;
 	u32 repeat = kattr->test.repeat;
 	u32 retval, duration;
+	int hh_len = ETH_HLEN;
 	struct sk_buff *skb;
 	void *data;
 	int ret;
@@ -131,12 +132,22 @@ int bpf_prog_test_run_skb(struct bpf_pro
 	skb_reset_network_header(skb);
 
 	if (is_l2)
-		__skb_push(skb, ETH_HLEN);
+		__skb_push(skb, hh_len);
 	if (is_direct_pkt_access)
 		bpf_compute_data_end(skb);
 	retval = bpf_test_run(prog, skb, repeat, &duration);
-	if (!is_l2)
-		__skb_push(skb, ETH_HLEN);
+	if (!is_l2) {
+		if (skb_headroom(skb) < hh_len) {
+			int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb));
+
+			if (pskb_expand_head(skb, nhead, 0, GFP_USER)) {
+				kfree_skb(skb);
+				return -ENOMEM;
+			}
+		}
+		memset(__skb_push(skb, hh_len), 0, hh_len);
+	}
+
 	size = skb->len;
 	/* bpf program can never convert linear skb to non-linear */
 	if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -4335,6 +4335,24 @@ static struct bpf_test tests[] = {
 		.prog_type = BPF_PROG_TYPE_LWT_XMIT,
 	},
 	{
+		"make headroom for LWT_XMIT",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+			BPF_MOV64_IMM(BPF_REG_2, 34),
+			BPF_MOV64_IMM(BPF_REG_3, 0),
+			BPF_EMIT_CALL(BPF_FUNC_skb_change_head),
+			/* split for s390 to succeed */
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+			BPF_MOV64_IMM(BPF_REG_2, 42),
+			BPF_MOV64_IMM(BPF_REG_3, 0),
+			BPF_EMIT_CALL(BPF_FUNC_skb_change_head),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_LWT_XMIT,
+	},
+	{
 		"invalid access of tc_classid for LWT_IN",
 		.insns = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,



  parent reply	other threads:[~2021-12-20 14:41 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-20 14:33 [PATCH 4.14 00/45] 4.14.259-rc1 review Greg Kroah-Hartman
2021-12-20 14:33 ` [PATCH 4.14 01/45] nfc: fix segfault in nfc_genl_dump_devices_done Greg Kroah-Hartman
2021-12-20 14:33 ` [PATCH 4.14 02/45] drm/msm/dsi: set default num_data_lanes Greg Kroah-Hartman
2021-12-20 14:33 ` [PATCH 4.14 03/45] net/mlx4_en: Update reported link modes for 1/10G Greg Kroah-Hartman
2021-12-20 14:33 ` [PATCH 4.14 04/45] parisc/agp: Annotate parisc agp init functions with __init Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 05/45] i2c: rk3x: Handle a spurious start completion interrupt flag Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 06/45] net: netlink: af_netlink: Prevent empty skb by adding a check on len Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 07/45] tracing: Fix a kmemleak false positive in tracing_map Greg Kroah-Hartman
2021-12-20 14:34 ` Greg Kroah-Hartman [this message]
2021-12-20 14:34 ` [PATCH 4.14 09/45] hwmon: (dell-smm) Fix warning on /proc/i8k creation error Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 10/45] mac80211: send ADDBA requests using the tid/queue of the aggregation session Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 11/45] recordmcount.pl: look for jgnop instruction as well as bcrl on s390 Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 12/45] dm btree remove: fix use after free in rebalance_children() Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 13/45] audit: improve robustness of the audit queue handling Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 14/45] nfsd: fix use-after-free due to delegation race Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 15/45] x86: Make ARCH_USE_MEMREMAP_PROT a generic Kconfig symbol Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 16/45] x86/sme: Explicitly map new EFI memmap table as encrypted Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 17/45] hv: utils: add PTP_1588_CLOCK to Kconfig to fix build Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 18/45] ARM: socfpga: dts: fix qspi node compatible Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 19/45] dmaengine: st_fdma: fix MODULE_ALIAS Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 20/45] soc/tegra: fuse: Fix bitwise vs. logical OR warning Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 21/45] igbvf: fix double free in `igbvf_probe` Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 22/45] ixgbe: set X550 MDIO speed before talking to PHY Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 23/45] net/packet: rx_owner_map depends on pg_vec Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 24/45] sit: do not call ipip6_dev_free() from sit_init_net() Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 25/45] USB: gadget: bRequestType is a bitfield, not a enum Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 26/45] PCI/MSI: Clear PCI_MSIX_FLAGS_MASKALL on error Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 27/45] PCI/MSI: Mask MSI-X vectors only on success Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 28/45] USB: serial: option: add Telit FN990 compositions Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 29/45] timekeeping: Really make sure wall_to_monotonic isnt positive Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 30/45] libata: if T_LENGTH is zero, dma direction should be DMA_NONE Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 31/45] net: systemport: Add global locking for descriptor lifecycle Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 32/45] firmware: arm_scpi: Fix string overflow in SCPI genpd driver Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 33/45] ARM: dts: imx6ull-pinfunc: Fix CSI_DATA07__ESAI_TX0 pad name Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 34/45] fuse: annotate lock in fuse_reverse_inval_entry() Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 35/45] scsi: scsi_debug: Sanity check block descriptor length in resp_mode_select() Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 36/45] net: lan78xx: Avoid unnecessary self assignment Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 37/45] ARM: 8805/2: remove unneeded naked function usage Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 38/45] mwifiex: Remove unnecessary braces from HostCmd_SET_SEQ_NO_BSS_INFO Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 39/45] ARM: 8800/1: use choice for kernel unwinders Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 40/45] Input: touchscreen - avoid bitwise vs logical OR warning Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 41/45] xen/blkfront: harden blkfront against event channel storms Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 42/45] xen/netfront: harden netfront " Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 43/45] xen/console: harden hvc_xen " Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 44/45] xen/netback: fix rx queue stall detection Greg Kroah-Hartman
2021-12-20 14:34 ` [PATCH 4.14 45/45] xen/netback: dont queue unlimited number of packages Greg Kroah-Hartman
2021-12-20 18:25 ` [PATCH 4.14 00/45] 4.14.259-rc1 review Jon Hunter
2021-12-21 17:22 ` Naresh Kamboju
2021-12-21 23:12 ` Guenter Roeck

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=20211220143022.544815365@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ast@kernel.org \
    --cc=connoro@google.com \
    --cc=daniel@iogearbox.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+54f39d6ab58f39720a55@syzkaller.appspotmail.com \
    --cc=syzbot+709412e651e55ed96498@syzkaller.appspotmail.com \
    /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.