From patchwork Fri Nov 15 00:31:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13875788 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A2A141362 for ; Fri, 15 Nov 2024 00:31:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731630714; cv=none; b=LDlIzXRUp3QLvkN8NAhe/o+Fb/pWMpNhsWolI6vEJ2M5cYqnue7K24VcZVl3GpArAjhGKZ9l7o/mX4FbPbZwZqijeMG4ud2lFckiAN7lWfJE9XSrfeD/aotfmwLWltXTnyavk14sXhZQ92WbDFnyHDJNCSxecnQE04UyHUU7z+I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731630714; c=relaxed/simple; bh=LU3XarEKwd1387jdjxqg+xfc1xKz1LA795ie3WoNQCc=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=XcVxDsSTTpqyvOHttYjdRUnevKGps5BJJ+RpE0rcfiC5Z/QPlr0Iaz4684Br1VCbcFGpknNLJRnmGgEk2SF/HwAyc54ND4YBhQk3QXn5t+V0/bRjwTNRxxVWmHsW3FLQEWP8rrFN8Mw9lhuPBYyc+Pu3FxHMH7mMqsEI0wwcNyI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=XJCqfkxP; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XJCqfkxP" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E33E3C4CECD; Fri, 15 Nov 2024 00:31:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1731630714; bh=LU3XarEKwd1387jdjxqg+xfc1xKz1LA795ie3WoNQCc=; h=From:To:Cc:Subject:Date:From; b=XJCqfkxP3BRZCf4ECAMx134YF9cbeXg/nCapfD7yPTjK4OougmH6h8zQsO+gdoXqg S0cMhOxrct06YMSGzWRM5c85ZM6G04oRRmoknF+tol5fqk/wv5TltWDHtHo+SZYNBW mSvwDg6ECx8wR00weAWB11HjEt4RCDmAVh/3+GZt5tya/HLCWkFR6r6Sn9q85S2+Jo ZjuxPgfgvYbQbS1yfli+TeDIPnAqyIR4ROJ7W5wTmsKcq2VvM6lPNqrJw2qJ7F4yb/ i/7TUzbrRk21FP33Urph99GF5B3p1y4hUzCGW6Jh9IIrDJ2M5/EPmQgYf6e/x5dvk1 XFCv9pu8ZsT1w== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, Jakub Kicinski , syzbot+d4373fa8042c06cefa84@syzkaller.appspotmail.com, dsahern@kernel.org Subject: [PATCH net 1/2] netlink: fix false positive warning in extack during dumps Date: Thu, 14 Nov 2024 16:31:49 -0800 Message-ID: <20241115003150.733141-1-kuba@kernel.org> X-Mailer: git-send-email 2.47.0 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Commit under fixes extended extack reporting to dumps. It works under normal conditions, because extack errors are usually reported during ->start() or the first ->dump(), it's quite rare that the dump starts okay but fails later. If the dump does fail later, however, the input skb will already have the initiating message pulled, so checking if bad attr falls within skb->data will fail. Switch the check to using nlh, which is always valid. syzbot found a way to hit that scenario by filling up the receive queue. In this case we initiate a dump but don't call ->dump() until there is read space for an skb. RIP: 0010:netlink_ack_tlv_fill+0x1a8/0x560 net/netlink/af_netlink.c:2209 Call Trace: netlink_dump_done+0x513/0x970 net/netlink/af_netlink.c:2250 netlink_dump+0x91f/0xe10 net/netlink/af_netlink.c:2351 netlink_recvmsg+0x6bb/0x11d0 net/netlink/af_netlink.c:1983 sock_recvmsg_nosec net/socket.c:1051 [inline] sock_recvmsg+0x22f/0x280 net/socket.c:1073 __sys_recvfrom+0x246/0x3d0 net/socket.c:2267 __do_sys_recvfrom net/socket.c:2285 [inline] __se_sys_recvfrom net/socket.c:2281 [inline] __x64_sys_recvfrom+0xde/0x100 net/socket.c:2281 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f RIP: 0033:0x7ff37dd17a79 Reported-by: syzbot+d4373fa8042c06cefa84@syzkaller.appspotmail.com Fixes: 8af4f60472fc ("netlink: support all extack types in dumps") Signed-off-by: Jakub Kicinski --- CC: dsahern@kernel.org --- include/net/netlink.h | 13 +++++++++++++ net/netlink/af_netlink.c | 13 +++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/include/net/netlink.h b/include/net/netlink.h index db6af207287c..efd906aff22f 100644 --- a/include/net/netlink.h +++ b/include/net/netlink.h @@ -659,6 +659,19 @@ nlmsg_next(const struct nlmsghdr *nlh, int *remaining) return (struct nlmsghdr *) ((unsigned char *) nlh + totlen); } +/** + * nlmsg_addr_in_payload - address points to a byte within the message payload + * @nlh: netlink message header + * + * Returns: true if address is within the payload of the message + */ +static inline bool nlmsg_addr_in_payload(const struct nlmsghdr *nlh, + const void *addr) +{ + return addr >= nlmsg_data(nlh) && + addr - (const void *) nlh < nlh->nlmsg_len; +} + /** * nla_parse - Parse a stream of attributes into a tb buffer * @tb: destination array with maxtype+1 elements diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index f84aad420d44..b49c9d745239 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2177,8 +2177,7 @@ netlink_ack_tlv_len(struct netlink_sock *nlk, int err, } static void -netlink_ack_tlv_fill(struct sk_buff *in_skb, struct sk_buff *skb, - const struct nlmsghdr *nlh, int err, +netlink_ack_tlv_fill(struct sk_buff *skb, const struct nlmsghdr *nlh, int err, const struct netlink_ext_ack *extack) { if (extack->_msg) @@ -2191,8 +2190,7 @@ netlink_ack_tlv_fill(struct sk_buff *in_skb, struct sk_buff *skb, return; if (extack->bad_attr && - !WARN_ON((u8 *)extack->bad_attr < in_skb->data || - (u8 *)extack->bad_attr >= in_skb->data + in_skb->len)) + !WARN_ON(!nlmsg_addr_in_payload(nlh, extack->bad_attr))) WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_OFFS, (u8 *)extack->bad_attr - (const u8 *)nlh)); if (extack->policy) @@ -2202,8 +2200,7 @@ netlink_ack_tlv_fill(struct sk_buff *in_skb, struct sk_buff *skb, WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_MISS_TYPE, extack->miss_type)); if (extack->miss_nest && - !WARN_ON((u8 *)extack->miss_nest < in_skb->data || - (u8 *)extack->miss_nest > in_skb->data + in_skb->len)) + !WARN_ON(!nlmsg_addr_in_payload(nlh, extack->miss_nest))) WARN_ON(nla_put_u32(skb, NLMSGERR_ATTR_MISS_NEST, (u8 *)extack->miss_nest - (const u8 *)nlh)); } @@ -2232,7 +2229,7 @@ static int netlink_dump_done(struct netlink_sock *nlk, struct sk_buff *skb, if (extack_len) { nlh->nlmsg_flags |= NLM_F_ACK_TLVS; if (skb_tailroom(skb) >= extack_len) { - netlink_ack_tlv_fill(cb->skb, skb, cb->nlh, + netlink_ack_tlv_fill(skb, cb->nlh, nlk->dump_done_errno, extack); nlmsg_end(skb, nlh); } @@ -2491,7 +2488,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, } if (tlvlen) - netlink_ack_tlv_fill(in_skb, skb, nlh, err, extack); + netlink_ack_tlv_fill(skb, nlh, err, extack); nlmsg_end(skb, rep); From patchwork Fri Nov 15 00:31:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13875789 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0E2661E49F; Fri, 15 Nov 2024 00:31:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731630715; cv=none; b=O1fNZuEwJojsGSTU1o3m7ksRTKb9aCCZikF5M5Vg8chrl0eM5CbjAbx8qCHeVuyEUzj/dhMSKKTCnAx4Bz/ZC5bGol3jVCI6LQkhiqLCOoIGsBnyO3l7MxRf6UDf7g1Kw3lP/VFifADKbiFYnRyhmJaiJVn1g4It+T18I2ocEPk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731630715; c=relaxed/simple; bh=LdyZfhGLcAffeQWdZ9vyCIw4OGV0nYZirm1HjPWiJ4c=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=bU+hVQn5i6kv+FDG47pTzVwWuPDyAwjXoJSTfg3FBJmp0au70ag4tCdc5/hlBYZc+AbASRDs5onBkL0BW1ZbWz8XMbqY9KkrE0BJ5h2lu/+2JKY61cr4XVmT/uKSJNc2JiwIX9k+hBKSEc2nbHshxrRNQFV9uHtzXgcIp29PIrg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=nXwOrYMg; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="nXwOrYMg" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5CED8C4CED4; Fri, 15 Nov 2024 00:31:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1731630714; bh=LdyZfhGLcAffeQWdZ9vyCIw4OGV0nYZirm1HjPWiJ4c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nXwOrYMgGmyxfYqfFsUZXEUGR0kZH5McM15cOenYsvWgCzwWIyqst8hJYGSbEEfXI IcxiFRB9gAvOgZzAOFIh2IsQ0Kdj/FQBtQzDX5c78qwJU1op3siUY+H83HH4v105yU 4CttpV+2lgbYc1CJAv2+WPP+PMicar1U+Md6pZBN+DCWMEyqLHlskMFgXMeEJxMYFo XQrxFx0dWo2Ftq3+XYg1tcAuIe4526kUAkYmabUSVe4qMOfFf8C9f+QYdJtGa5B9AX oJOvYr0Zn1uDbVz5zeH1yEaqtQMZMMsXnGQL808PXmTBBFRr1PLh2g3rQHYepmfYqL XPoeKbekSewSA== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, Jakub Kicinski , linux-kselftest@vger.kernel.org Subject: [PATCH net 2/2] selftests: net: test extacks in netlink dumps Date: Thu, 14 Nov 2024 16:31:50 -0800 Message-ID: <20241115003150.733141-2-kuba@kernel.org> X-Mailer: git-send-email 2.47.0 In-Reply-To: <20241115003150.733141-1-kuba@kernel.org> References: <20241115003150.733141-1-kuba@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org Test that extacks in dumps work. The test fills up the receive buffer to test both the inline dump (as part of sendmsg()) and delayed one (run during recvmsg()). Use YNL helpers to parse the messages. We need to add the test to YNL file to make sure the right include path are used. Signed-off-by: Jakub Kicinski --- CC: linux-kselftest@vger.kernel.org --- tools/testing/selftests/net/Makefile | 3 +- tools/testing/selftests/net/netlink-dumps.c | 129 ++++++++++++++++++++ 2 files changed, 130 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index 5e86f7a51b43..c427ee32b193 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -78,7 +78,6 @@ TEST_PROGS += test_vxlan_vnifiltering.sh TEST_GEN_FILES += io_uring_zerocopy_tx TEST_PROGS += io_uring_zerocopy_tx.sh TEST_GEN_FILES += bind_bhash -TEST_GEN_PROGS += netlink-dumps TEST_GEN_PROGS += sk_bind_sendto_listen TEST_GEN_PROGS += sk_connect_zero_addr TEST_GEN_PROGS += sk_so_peek_off @@ -100,7 +99,7 @@ TEST_PROGS += bpf_offload.py # YNL files, must be before "include ..lib.mk" EXTRA_CLEAN += $(OUTPUT)/libynl.a -YNL_GEN_FILES := ncdevmem +YNL_GEN_FILES := ncdevmem netlink-dumps TEST_GEN_FILES += $(YNL_GEN_FILES) TEST_FILES := settings diff --git a/tools/testing/selftests/net/netlink-dumps.c b/tools/testing/selftests/net/netlink-dumps.c index 7ee6dcd334df..195febbf6a61 100644 --- a/tools/testing/selftests/net/netlink-dumps.c +++ b/tools/testing/selftests/net/netlink-dumps.c @@ -12,11 +12,140 @@ #include #include +#include +#include #include #include +#include #include "../kselftest_harness.h" +#include + +struct ext_ack { + int err; + + __u32 attr_offs; + __u32 miss_type; + __u32 miss_nest; + const char *str; +}; + +/* 0: no done, 1: done found, 2: extack found, -1: error */ +static int nl_get_extack(char *buf, size_t n, struct ext_ack *ea) +{ + const struct nlmsghdr *nlh; + const struct nlattr *attr; + ssize_t rem; + + for (rem = n; rem > 0; NLMSG_NEXT(nlh, rem)) { + nlh = (struct nlmsghdr *)&buf[n - rem]; + if (!NLMSG_OK(nlh, rem)) + return -1; + + if (nlh->nlmsg_type != NLMSG_DONE) + continue; + + ea->err = -*(int *)NLMSG_DATA(nlh); + + if (!(nlh->nlmsg_flags & NLM_F_ACK_TLVS)) + return 1; + + ynl_attr_for_each(attr, nlh, sizeof(int)) { + switch (ynl_attr_type(attr)) { + case NLMSGERR_ATTR_OFFS: + ea->attr_offs = ynl_attr_get_u32(attr); + break; + case NLMSGERR_ATTR_MISS_TYPE: + ea->miss_type = ynl_attr_get_u32(attr); + break; + case NLMSGERR_ATTR_MISS_NEST: + ea->miss_nest = ynl_attr_get_u32(attr); + break; + case NLMSGERR_ATTR_MSG: + ea->str = ynl_attr_get_str(attr); + break; + } + } + + return 2; + } + + return 0; +} + +static const struct { + struct nlmsghdr nlhdr; + struct ndmsg ndm; + struct nlattr ahdr; + __u32 val; +} dump_neigh_bad = { + .nlhdr = { + .nlmsg_len = sizeof(dump_neigh_bad), + .nlmsg_type = RTM_GETNEIGH, + .nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP, + .nlmsg_seq = 1, + }, + .ndm = { + .ndm_family = 123, + }, + .ahdr = { + .nla_len = 4 + 4, + .nla_type = NDA_FLAGS_EXT, + }, + .val = -1, // should fail MASK validation +}; + +TEST(dump_extack) +{ + int netlink_sock; + char buf[8192]; + int one = 1; + int i, cnt; + ssize_t n; + + netlink_sock = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE); + ASSERT_GE(netlink_sock, 0); + + n = setsockopt(netlink_sock, SOL_NETLINK, NETLINK_CAP_ACK, + &one, sizeof(one)); + ASSERT_EQ(n, 0); + n = setsockopt(netlink_sock, SOL_NETLINK, NETLINK_EXT_ACK, + &one, sizeof(one)); + ASSERT_EQ(n, 0); + n = setsockopt(netlink_sock, SOL_NETLINK, NETLINK_GET_STRICT_CHK, + &one, sizeof(one)); + ASSERT_EQ(n, 0); + + /* Dump so many times we fill up the buffer */ + cnt = 64; + for (i = 0; i < cnt; i++) { + n = send(netlink_sock, &dump_neigh_bad, + sizeof(dump_neigh_bad), 0); + ASSERT_EQ(n, sizeof(dump_neigh_bad)); + } + + /* Read out the ENOBUFS */ + n = recv(netlink_sock, buf, sizeof(buf), MSG_DONTWAIT); + EXPECT_EQ(n, -1); + EXPECT_EQ(errno, ENOBUFS); + + for (i = 0; i < cnt; i++) { + struct ext_ack ea = {}; + + n = recv(netlink_sock, buf, sizeof(buf), MSG_DONTWAIT); + if (n < 0) { + ASSERT_GE(i, 10); + break; + } + ASSERT_GE(n, (ssize_t)sizeof(struct nlmsghdr)); + + EXPECT_EQ(nl_get_extack(buf, n, &ea), 2); + EXPECT_EQ(ea.attr_offs, + sizeof(struct nlmsghdr) + sizeof(struct ndmsg)); + } +} + static const struct { struct nlmsghdr nlhdr; struct genlmsghdr genlhdr;