Skip to content

Commit 528269f

Browse files
author
Paolo Abeni
committed
Daniel Borkmann says: ==================== pull-request: bpf 2024-07-09 The following pull-request contains BPF updates for your *net* tree. We've added 3 non-merge commits during the last 1 day(s) which contain a total of 5 files changed, 81 insertions(+), 11 deletions(-). The main changes are: 1) Fix a use-after-free in a corner case where tcx_entry got released too early. Also add BPF test coverage along with the fix, from Daniel Borkmann. 2) Fix a kernel panic on Loongarch in sk_msg_recvmsg() which got triggered by running BPF sockmap selftests, from Geliang Tang. bpf-for-netdev * tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: skmsg: Skip zero length skb in sk_msg_recvmsg selftests/bpf: Extend tcx tests to cover late tcx_entry release bpf: Fix too early release of tcx_entry ==================== Link: https://patch.msgid.link/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
2 parents 0913ec3 + f0c1802 commit 528269f

File tree

5 files changed

+81
-11
lines changed

5 files changed

+81
-11
lines changed

include/net/tcx.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ struct mini_Qdisc;
1313
struct tcx_entry {
1414
struct mini_Qdisc __rcu *miniq;
1515
struct bpf_mprog_bundle bundle;
16-
bool miniq_active;
16+
u32 miniq_active;
1717
struct rcu_head rcu;
1818
};
1919

@@ -125,11 +125,16 @@ static inline void tcx_skeys_dec(bool ingress)
125125
tcx_dec();
126126
}
127127

128-
static inline void tcx_miniq_set_active(struct bpf_mprog_entry *entry,
129-
const bool active)
128+
static inline void tcx_miniq_inc(struct bpf_mprog_entry *entry)
130129
{
131130
ASSERT_RTNL();
132-
tcx_entry(entry)->miniq_active = active;
131+
tcx_entry(entry)->miniq_active++;
132+
}
133+
134+
static inline void tcx_miniq_dec(struct bpf_mprog_entry *entry)
135+
{
136+
ASSERT_RTNL();
137+
tcx_entry(entry)->miniq_active--;
133138
}
134139

135140
static inline bool tcx_entry_is_active(struct bpf_mprog_entry *entry)

net/core/skmsg.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,8 @@ int sk_msg_recvmsg(struct sock *sk, struct sk_psock *psock, struct msghdr *msg,
434434
page = sg_page(sge);
435435
if (copied + copy > len)
436436
copy = len - copied;
437-
copy = copy_page_to_iter(page, sge->offset, copy, iter);
437+
if (copy)
438+
copy = copy_page_to_iter(page, sge->offset, copy, iter);
438439
if (!copy) {
439440
copied = copied ? copied : -EFAULT;
440441
goto out;

net/sched/sch_ingress.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ static int ingress_init(struct Qdisc *sch, struct nlattr *opt,
9191
entry = tcx_entry_fetch_or_create(dev, true, &created);
9292
if (!entry)
9393
return -ENOMEM;
94-
tcx_miniq_set_active(entry, true);
94+
tcx_miniq_inc(entry);
9595
mini_qdisc_pair_init(&q->miniqp, sch, &tcx_entry(entry)->miniq);
9696
if (created)
9797
tcx_entry_update(dev, entry, true);
@@ -121,7 +121,7 @@ static void ingress_destroy(struct Qdisc *sch)
121121
tcf_block_put_ext(q->block, sch, &q->block_info);
122122

123123
if (entry) {
124-
tcx_miniq_set_active(entry, false);
124+
tcx_miniq_dec(entry);
125125
if (!tcx_entry_is_active(entry)) {
126126
tcx_entry_update(dev, NULL, true);
127127
tcx_entry_free(entry);
@@ -257,7 +257,7 @@ static int clsact_init(struct Qdisc *sch, struct nlattr *opt,
257257
entry = tcx_entry_fetch_or_create(dev, true, &created);
258258
if (!entry)
259259
return -ENOMEM;
260-
tcx_miniq_set_active(entry, true);
260+
tcx_miniq_inc(entry);
261261
mini_qdisc_pair_init(&q->miniqp_ingress, sch, &tcx_entry(entry)->miniq);
262262
if (created)
263263
tcx_entry_update(dev, entry, true);
@@ -276,7 +276,7 @@ static int clsact_init(struct Qdisc *sch, struct nlattr *opt,
276276
entry = tcx_entry_fetch_or_create(dev, false, &created);
277277
if (!entry)
278278
return -ENOMEM;
279-
tcx_miniq_set_active(entry, true);
279+
tcx_miniq_inc(entry);
280280
mini_qdisc_pair_init(&q->miniqp_egress, sch, &tcx_entry(entry)->miniq);
281281
if (created)
282282
tcx_entry_update(dev, entry, false);
@@ -302,15 +302,15 @@ static void clsact_destroy(struct Qdisc *sch)
302302
tcf_block_put_ext(q->egress_block, sch, &q->egress_block_info);
303303

304304
if (ingress_entry) {
305-
tcx_miniq_set_active(ingress_entry, false);
305+
tcx_miniq_dec(ingress_entry);
306306
if (!tcx_entry_is_active(ingress_entry)) {
307307
tcx_entry_update(dev, NULL, true);
308308
tcx_entry_free(ingress_entry);
309309
}
310310
}
311311

312312
if (egress_entry) {
313-
tcx_miniq_set_active(egress_entry, false);
313+
tcx_miniq_dec(egress_entry);
314314
if (!tcx_entry_is_active(egress_entry)) {
315315
tcx_entry_update(dev, NULL, false);
316316
tcx_entry_free(egress_entry);

tools/testing/selftests/bpf/config

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,12 @@ CONFIG_MPLS=y
5858
CONFIG_MPLS_IPTUNNEL=y
5959
CONFIG_MPLS_ROUTING=y
6060
CONFIG_MPTCP=y
61+
CONFIG_NET_ACT_SKBMOD=y
62+
CONFIG_NET_CLS=y
6163
CONFIG_NET_CLS_ACT=y
6264
CONFIG_NET_CLS_BPF=y
6365
CONFIG_NET_CLS_FLOWER=y
66+
CONFIG_NET_CLS_MATCHALL=y
6467
CONFIG_NET_FOU=y
6568
CONFIG_NET_FOU_IP_TUNNELS=y
6669
CONFIG_NET_IPGRE=y

tools/testing/selftests/bpf/prog_tests/tc_links.c

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#define ping_cmd "ping -q -c1 -w1 127.0.0.1 > /dev/null"
1010

1111
#include "test_tc_link.skel.h"
12+
13+
#include "netlink_helpers.h"
1214
#include "tc_helpers.h"
1315

1416
void serial_test_tc_links_basic(void)
@@ -1787,6 +1789,65 @@ void serial_test_tc_links_ingress(void)
17871789
test_tc_links_ingress(BPF_TCX_INGRESS, false, false);
17881790
}
17891791

1792+
struct qdisc_req {
1793+
struct nlmsghdr n;
1794+
struct tcmsg t;
1795+
char buf[1024];
1796+
};
1797+
1798+
static int qdisc_replace(int ifindex, const char *kind, bool block)
1799+
{
1800+
struct rtnl_handle rth = { .fd = -1 };
1801+
struct qdisc_req req;
1802+
int err;
1803+
1804+
err = rtnl_open(&rth, 0);
1805+
if (!ASSERT_OK(err, "open_rtnetlink"))
1806+
return err;
1807+
1808+
memset(&req, 0, sizeof(req));
1809+
req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg));
1810+
req.n.nlmsg_flags = NLM_F_CREATE | NLM_F_REPLACE | NLM_F_REQUEST;
1811+
req.n.nlmsg_type = RTM_NEWQDISC;
1812+
req.t.tcm_family = AF_UNSPEC;
1813+
req.t.tcm_ifindex = ifindex;
1814+
req.t.tcm_parent = 0xfffffff1;
1815+
1816+
addattr_l(&req.n, sizeof(req), TCA_KIND, kind, strlen(kind) + 1);
1817+
if (block)
1818+
addattr32(&req.n, sizeof(req), TCA_INGRESS_BLOCK, 1);
1819+
1820+
err = rtnl_talk(&rth, &req.n, NULL);
1821+
ASSERT_OK(err, "talk_rtnetlink");
1822+
rtnl_close(&rth);
1823+
return err;
1824+
}
1825+
1826+
void serial_test_tc_links_dev_chain0(void)
1827+
{
1828+
int err, ifindex;
1829+
1830+
ASSERT_OK(system("ip link add dev foo type veth peer name bar"), "add veth");
1831+
ifindex = if_nametoindex("foo");
1832+
ASSERT_NEQ(ifindex, 0, "non_zero_ifindex");
1833+
err = qdisc_replace(ifindex, "ingress", true);
1834+
if (!ASSERT_OK(err, "attaching ingress"))
1835+
goto cleanup;
1836+
ASSERT_OK(system("tc filter add block 1 matchall action skbmod swap mac"), "add block");
1837+
err = qdisc_replace(ifindex, "clsact", false);
1838+
if (!ASSERT_OK(err, "attaching clsact"))
1839+
goto cleanup;
1840+
/* Heuristic: kern_sync_rcu() alone does not work; a wait-time of ~5s
1841+
* triggered the issue without the fix reliably 100% of the time.
1842+
*/
1843+
sleep(5);
1844+
ASSERT_OK(system("tc filter add dev foo ingress matchall action skbmod swap mac"), "add filter");
1845+
cleanup:
1846+
ASSERT_OK(system("ip link del dev foo"), "del veth");
1847+
ASSERT_EQ(if_nametoindex("foo"), 0, "foo removed");
1848+
ASSERT_EQ(if_nametoindex("bar"), 0, "bar removed");
1849+
}
1850+
17901851
static void test_tc_links_dev_mixed(int target)
17911852
{
17921853
LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 1);

0 commit comments

Comments
 (0)