mirror of
https://github.com/torvalds/linux.git
synced 2026-11-06 21:58:01 +01:00
Merge branch 'netlink-fix-a-dump-livelock-with-a-notification-queued'
Jhonata Poma-Hansen says:
====================
netlink: fix a dump livelock with a notification queued
A netlink dump that starts while a notification is still charged to a
socket with a small receive buffer fails with ENOBUFS, and every retry
from recvmsg() fails the same way. A reader that peeks before reading,
as libnl does, never consumes the message at the head of the queue and
spins forever. I hit this with "iw dev wlan0 scan" on an arm64 board.
Patch 1 goes back to checking sk_rcvbuf before charging the dump skb,
as before commit ae8f160e7e. Patch 2 adds a selftest; it fails
without patch 1 and is skipped without the privileges to create a
netns.
====================
Link: https://patch.msgid.link/20261027-netlink-dump-livelock-v2-0-c4bb05352a33@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
commit
5bc3aaaa2d
|
|
@ -2322,9 +2322,13 @@ static int netlink_dump(struct sock *sk, bool lock_taken)
|
|||
if (!skb)
|
||||
goto errout_skb;
|
||||
|
||||
/* Check the limit before charging the skb: netlink_recvmsg() continues
|
||||
* the dump at or below half of sk_rcvbuf, and the skb must be queued
|
||||
* then. Keep sk_rmem_alloc at or below INT_MAX.
|
||||
*/
|
||||
rcvbuf = READ_ONCE(sk->sk_rcvbuf);
|
||||
rmem = atomic_add_return(skb->truesize, &sk->sk_rmem_alloc);
|
||||
if (rmem != skb->truesize && rmem >= rcvbuf) {
|
||||
if (rmem - skb->truesize >= rcvbuf || rmem > INT_MAX) {
|
||||
atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
|
||||
goto errout_skb;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,9 +2,12 @@
|
|||
|
||||
#define _GNU_SOURCE
|
||||
|
||||
#include <errno.h>
|
||||
#include <fcntl.h>
|
||||
#include <sched.h>
|
||||
#include <stdio.h>
|
||||
#include <string.h>
|
||||
#include <sys/ioctl.h>
|
||||
#include <sys/socket.h>
|
||||
#include <sys/stat.h>
|
||||
#include <sys/syscall.h>
|
||||
|
|
@ -260,4 +263,103 @@ TEST(close_with_ref)
|
|||
usleep(100 * 1000);
|
||||
}
|
||||
|
||||
static int lo_set_up(int sock, bool up)
|
||||
{
|
||||
struct ifreq ifr = {};
|
||||
|
||||
strcpy(ifr.ifr_name, "lo");
|
||||
if (ioctl(sock, SIOCGIFFLAGS, &ifr))
|
||||
return -1;
|
||||
if (up)
|
||||
ifr.ifr_flags |= IFF_UP;
|
||||
else
|
||||
ifr.ifr_flags &= ~IFF_UP;
|
||||
return ioctl(sock, SIOCSIFFLAGS, &ifr);
|
||||
}
|
||||
|
||||
static const struct {
|
||||
struct nlmsghdr nlhdr;
|
||||
struct ifinfomsg ifm;
|
||||
} dump_links = {
|
||||
.nlhdr = {
|
||||
.nlmsg_len = sizeof(dump_links),
|
||||
.nlmsg_type = RTM_GETLINK,
|
||||
.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP,
|
||||
.nlmsg_seq = 1,
|
||||
},
|
||||
.ifm = {
|
||||
.ifi_family = AF_UNSPEC,
|
||||
},
|
||||
};
|
||||
|
||||
// A dump started while a notification is still queued must make progress
|
||||
// for a reader which peeks before reading, like libnl does
|
||||
TEST(dump_with_queued_ntf)
|
||||
{
|
||||
struct sockaddr_nl addr = {
|
||||
.nl_family = AF_NETLINK,
|
||||
.nl_groups = RTMGRP_LINK,
|
||||
};
|
||||
int netlink_sock, inet_sock, i, ret, links = 0;
|
||||
int rcvbuf = 2048;
|
||||
bool done = false;
|
||||
char buf[32768];
|
||||
ssize_t n;
|
||||
|
||||
ret = unshare(CLONE_NEWNET);
|
||||
if (ret && errno == EPERM)
|
||||
SKIP(return, "Needs CAP_SYS_ADMIN for a netns");
|
||||
ASSERT_EQ(ret, 0);
|
||||
|
||||
inet_sock = socket(AF_INET, SOCK_DGRAM, 0);
|
||||
ASSERT_GE(inet_sock, 0);
|
||||
netlink_sock = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
|
||||
ASSERT_GE(netlink_sock, 0);
|
||||
ASSERT_EQ(setsockopt(netlink_sock, SOL_SOCKET, SO_RCVBUF,
|
||||
&rcvbuf, sizeof(rcvbuf)), 0);
|
||||
ASSERT_EQ(bind(netlink_sock, (struct sockaddr *)&addr,
|
||||
sizeof(addr)), 0);
|
||||
|
||||
// A large read makes the kernel attempt large dump skbs
|
||||
ASSERT_EQ(lo_set_up(inet_sock, true), 0);
|
||||
n = recv(netlink_sock, buf, sizeof(buf), 0);
|
||||
ASSERT_GE(n, (ssize_t)sizeof(struct nlmsghdr));
|
||||
|
||||
// Leave one notification queued, then start the dump
|
||||
ASSERT_EQ(lo_set_up(inet_sock, false), 0);
|
||||
n = send(netlink_sock, &dump_links, sizeof(dump_links), 0);
|
||||
ASSERT_EQ(n, sizeof(dump_links));
|
||||
|
||||
for (i = 0; i < 100 && !done; i++) {
|
||||
struct nlmsghdr *nlh;
|
||||
|
||||
n = recv(netlink_sock, buf, sizeof(buf),
|
||||
MSG_PEEK | MSG_TRUNC | MSG_DONTWAIT);
|
||||
if (n < 0)
|
||||
continue;
|
||||
n = recv(netlink_sock, buf, sizeof(buf), MSG_DONTWAIT);
|
||||
if (n < 0)
|
||||
continue;
|
||||
|
||||
for (nlh = (struct nlmsghdr *)buf; NLMSG_OK(nlh, n);
|
||||
nlh = NLMSG_NEXT(nlh, n)) {
|
||||
if (nlh->nlmsg_seq != dump_links.nlhdr.nlmsg_seq)
|
||||
continue;
|
||||
if (nlh->nlmsg_type == NLMSG_ERROR)
|
||||
EXPECT_EQ(*(int *)NLMSG_DATA(nlh), 0);
|
||||
if (nlh->nlmsg_type == RTM_NEWLINK)
|
||||
links++;
|
||||
if (nlh->nlmsg_type == NLMSG_DONE) {
|
||||
EXPECT_EQ(*(int *)NLMSG_DATA(nlh), 0);
|
||||
done = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
EXPECT_TRUE(done);
|
||||
EXPECT_GE(links, 1);
|
||||
|
||||
close(netlink_sock);
|
||||
close(inet_sock);
|
||||
}
|
||||
|
||||
TEST_HARNESS_MAIN
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user