| Age | Commit message (Collapse) | Author |
|---|
|
PIDFD_GET_INFO describes the task while there is one, and with
PIDFD_INFO_EXIT its exit status once it has been reaped. pidfd_info()
looks at the two in the wrong order and can miss both:
pidfd_info() release_task()
PIDFS_ATTR_BIT_EXIT is not set
pidfs_exit()
__unhash_process()
get_pid_task() == NULL
goto copy_out
The ioctl succeeds with neither PIDFD_INFO_PID nor PIDFD_INFO_EXIT in the
mask. If the task is unhashed a little later, after get_pid_task(), its
pid reads as 0 and the ioctl fails with -ESRCH, although the caller asked
for the exit information and it is available by now.
The second case does not even need an exit. When a sub-thread execs,
de_thread() hands the leader's pid over to it and releases the old leader.
A caller that got hold of the old leader reports the tid of the sub-thread
as the pid, or -ESRCH for a process that is alive.
Look up the task first. pidfs_exit() runs before the task is unhashed, so
a caller that finds no task finds the exit information; this needs a
barrier on both sides because taking tasklist_lock does not order the
store in pidfs_exit() against the one in detach_pid(). And start over if
the task no longer owns the pid when all the fields have been read: then
there is either exit information or another task.
Go by the pid for that check and for the pid we report, not by
->thread_pid of the task. exchange_tids() moves the pid to the exec'ing
thread before it updates ->thread_pid of either task. A caller that
finds the new leader in between would read the old tid of the sub-thread
from ->thread_pid, and a check of ->thread_pid would restart until the
exec'ing thread gets to its store. On PREEMPT_RT that thread can be
preempted with tasklist_lock held and can't move to another CPU. If the
RT task that preempted it calls PIDFD_GET_INFO, the ioctl spins until
the exec'ing thread gets the CPU back, and that never happens if the
exec'ing thread is an RT task of lower priority.
Christian Brauner <brauner@kernel.org> says:
Go by @pid instead of ->thread_pid of the task for the reported pid and
for the restart check.
Fixes: 7477d7dce48a ("pidfs: allow to retrieve exit information")
Fixes: cdda1f26e74b ("pidfd: add ioctl to retrieve pid info")
Cc: stable@vger.kernel.org
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
listns() resumes after the last returned id by looking up the first
namespace with a larger id with lookup_ns_id_after(), or with
lookup_ns_owner_after() when filtering by owner. Both also take a
reference with ns_get_unless_inactive(). If that namespace is inactive,
e.g. a network namespace that is only pinned by a socket or a user
namespace that is only pinned by a file's credentials, or if it is being
torn down, the lookup returns NULL and listns() fails with -ENOENT.
Userspace takes that as the end of the list, so every namespace after
it is silently missed:
ids 10 (active), 11 (inactive), 12 (active)
listns(ns_id = 0, nr = 1) -> 10
listns(ns_id = 10, nr = 1) -> lookup_ns_id_after(10) -> NULL, -ENOENT
12 is never listed
A call without a cursor doesn't have this problem because the loop just
skips the namespaces that legitimize_ns() rejects.
The reference only kept the starting point valid until the RCU walk
began. Look it up under rcu_read_lock() instead, without taking a
reference. The namespace can't be freed before the RCU read-side section
ends, and if it is unlinked in the meantime its ->next still leads back
into the list, which the walk already relies on for every other entry.
The starting namespace then goes through legitimize_ns() like every
other one, and kls->first_ns isn't needed anymore.
Reported-by: shaos <shawnq1@163.com>
Link: https://lore.kernel.org/r/20261020110746.959806-1-shawnq1@163.com
Fixes: 76b6f5dfb3fd ("nstree: add listns()")
Cc: stable@vger.kernel.org
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
tcp_set_default_congestion_control() takes a reference to an algorithm
before checking whether a non-init network namespace may select it. A
rejected request returns -EPERM without dropping the reference.
This affects module-backed algorithms too; BPF is not required. For
example, after root loads tcp_bbr, a user with CAP_NET_ADMIN in a
non-init network namespace can repeatedly request it as the default
although it is not in tcp_allowed_congestion_control. Each rejected
write leaks a module reference. Since module->refcnt is an atomic_t,
2^31 such writes can make it negative; later module_put() calls warn,
and rmmod can hit the BUG_ON() in try_release_module_ref().
Check the namespace restriction before acquiring the reference, as
tcp_set_congestion_control() already does.
Fixes: 8d432592f30f ("net: Only allow init netns to set default tcp cong to a restricted algo")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Suggested-by: Eric Dumazet <edumazet@kernel.org>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Link: https://patch.msgid.link/71c142b2d45af6a2fe7fc6cd4a5ca52c53afbe23.1793075984.git.zhilinz@nebusec.ai
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
With -r (SOCK_RAW, IPPROTO_UDP) the test writes only the UDP header and
payload, and the kernel adds the IP header. get_udp_csum() still expects
the IP source and destination addresses to directly precede the UDP
header, and reads 2 * alen bytes before the start of the buffer: 8 bytes
for IPv4, 32 bytes for IPv6.
ASan reports a heap-buffer-overflow for both -4 -r and -6 -r. Without
ASan the read usually goes unnoticed, but it faults when the buffer is
the first chunk in the heap. That happens on Ubuntu 26.04, where the
kselftest runner wraps the test in uutils stdbuf, and txtimestamp.sh
fails with SIGSEGV on every run.
Even without a crash, -r packets were sent with a checksum computed
over unrelated heap memory, so their UDP checksum was invalid.
Build the pseudo-header from the destination address and checksum only
the UDP header and payload. -R and -P already write daddr as both the
source and destination address, so their checksums are unchanged. With
-r the kernel selects the source address, which equals daddr for the
loopback addresses used by txtimestamp.sh.
Fixes: b52354aa068e ("selftests: expand txtimestamp with ipv6 dgram + raw and pf_packet")
Signed-off-by: Jamie Nguyen <jamien@nvidia.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
Link: https://patch.msgid.link/20261027162945.2122306-1-jamien@nvidia.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The gtp generic netlink family sends notifications on its "gtp"
multicast group when a PDP context is added or deleted, and for GTP-U
echo responses. They carry the same attributes as a GETPDP reply: the
MS and peer addresses, and the TEIDs, or the TID and the flow label.
GETPDP requires CAP_NET_ADMIN, but generic netlink lets any user join a
multicast group by default, so an unprivileged user in the network
namespace of the gtp device can read all of this. Found by code
inspection.
Fix by marking the group with GENL_MCAST_CAP_NET_ADMIN, like the psample
"packets" group, and with uns-admin-perm in the gtp netlink spec. Only
users with CAP_NET_ADMIN in the user namespace owning the network
namespace can then join it.
This is still weaker than the check on GETPDP, which needs
CAP_NET_ADMIN in the initial user namespace: in a network namespace
owned by another user namespace, its root can join the group, but not
use GETPDP. Genetlink has no multicast group flag for the initial user
namespace, and ioam6 pairs its ops and its group the same way. This fix
blocks the unprivileged users.
Tested by joining the group as root, as root without CAP_NET_ADMIN, and
as an unprivileged user:
Before:
root: joined
root without CAP_NET_ADMIN: joined
nobody: joined
After:
root: joined
root without CAP_NET_ADMIN: EPERM
nobody: EPERM
Fixes: 50aba46c234e ("gtp: add notification mechanism")
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Anil Kaushik <anilkaushikwireless@gmail.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20261027130623.3136658-1-anilkaushikwireless@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
simple_pin_fs() takes a reference on the internal mount for every pin
and simple_release_fs() drops it again. So every debugfs, tracefs,
securityfs, configfs, apparmorfs, drm and vfio file that is created and
removed costs a mntget() and a mntput() on that mount.
The mount comes from vfs_kern_mount() and not from kern_mount(), so it
never gets MNT_NS_INTERNAL and mntput_no_expire() can't take the fast
path for it. Each of those mntput()s ends up in
mntput_no_expire_slowpath(), takes mount_lock and sums the per-cpu
counters in mnt_get_count() to find out whether it dropped the last
reference. It never did for anything but the last pin, that's what the
pin count is for.
The reference per pin never bought anything. The pin count under
pin_fs_lock is what keeps the mount alive, nothing looks at the mount's
refcount to find out how many pins there are and the mount is only torn
down once the last pin is gone.
Keep the reference vfs_kern_mount() returned as the one reference the
pins hold. Further pins only bump the count and the last
simple_release_fs() drops the reference. The refcount of a pinned mount
now changes when it is first pinned and when the last pin goes away.
Everything in between is a spinlock and a counter.
Reported-by: kernel test robot <yi1.lai@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202610130959.d3ca4cfa-lkp@intel.com
Link: https://patch.msgid.link/20261026-work-libfs-pin_fs-v1-1-99ef52ee10a3@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
make W=1 reports excess kernel-doc parameters for bio_copy_kern()
and blk_rq_integrity_dma_map_iter_next():
block/blk-map.c:366: warning: Excess function parameter 'op' description in 'bio_copy_kern'
block/blk-mq-dma.c:379: warning: Excess function parameter 'state' description in 'blk_rq_integrity_dma_map_iter_next'
bio_copy_kern() derives the operation from @rq and has no @op argument,
and blk_rq_integrity_dma_map_iter_next() lost its @state argument in
commit 91e1c1bcf0f2 ("block, nvme: remove unused dma_iova_state function
parameter"). Drop the stale parameter descriptions.
While at it, fix the "ft" typo in the return value description of
blk_rq_dma_map_iter_start() and blk_rq_dma_map_iter_next().
Signed-off-by: Yuehong Wu <wuyuehong@cmss.chinamobile.com>
Link: https://patch.msgid.link/20261029091047.3085558-1-wuyuehong@cmss.chinamobile.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
proc_net_d_revalidate() always returns 0 so that a dentry below
/proc/<pid>/net is never reused after setns(). So lookup_fast() will end
up calling d_invalidate(), which unhashes the dentry and detaches every
mount on it, in every mount namespace; including locked mounts.
Then lookup_slow() creates a new dentry for the same entry. So a mount
on /proc/<pid>/net/dev succeeds and goes away with the first lookup of
the name by anybody with search permission on the directory. Given it's
0555 that's everyone's grandma. A mask on such a name hides nothing as
the repro below shows.
Treat those entries like I did fd/, fdinfo/ and map_files/ and refuse to
mount on them in the first place. It's nonsensical theater otherwise:
procnet net/dev stat
// SPDX-License-Identifier: GPL-2.0
/*
* procnet: cover an entry of /proc/<pid>/ with a bind mount, lock the mount,
* then look the name up.
*
* The first task stays alive, <pid> is its pid. The "container" task has its
* own mount namespace. There it binds a mask on /proc/<pid>/<name>. It drops to
* uid 1000 and enters a new user namespace with a new mount namespace, so that
* the mask is locked. Then one task looks the name up:
*
* stat the container calls stat() on the name
* umount the container calls umount2(MNT_DETACH) on the name
* nobody a task with uid 65534 in the first namespaces calls stat()
*
* The container lists its mounts before and after (that does not look the
* name up) and then reads the name. MASK is fine. Anything else means that
* the lookup uncovered the entry.
*
* usage: procnet <name> stat|umount|nobody
* name "status" is the control, "net/dev" is a file below net, "net/stat" is
* a directory below net
*/
#define _GNU_SOURCE
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <sched.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>
#include <sys/mount.h>
#include <sys/prctl.h>
#include <sys/stat.h>
#include <sys/wait.h>
#define WORK "/run/pn"
static char target[128];
static void die(const char *what)
{
fprintf(stderr, "FATAL %s: %s\n", what, strerror(errno));
exit(2);
}
static void write_str(const char *path, const char *s, int flags)
{
int fd = open(path, O_WRONLY | flags, 0644);
if (fd < 0 || write(fd, s, strlen(s)) < 0)
die(path);
close(fd);
}
/* list the mounts below /proc without a lookup of the target */
static void mounts(const char *when)
{
char line[512];
FILE *f = fopen("/proc/self/mountinfo", "r");
int n = 0;
if (!f)
die("mountinfo");
while (fgets(line, sizeof(line), f)) {
if (!strstr(line, " /proc/"))
continue;
printf("container: %s: mount: %s", when, line);
n++;
}
fclose(f);
if (!n)
printf("container: %s: nothing is mounted below /proc\n", when);
}
static void show(const char *when, int isdir)
{
char buf[48] = "";
int fd;
if (isdir) {
struct dirent *de;
DIR *d = opendir(target);
int hit = 0;
if (!d) {
printf("container: %s: opendir(%s) = %s\n", when, target, strerror(errno));
return;
}
printf("container: %s: readdir(%s) =", when, target);
while ((de = readdir(d))) {
if (de->d_name[0] == '.')
continue;
printf(" %s", de->d_name);
hit |= strcmp(de->d_name, "MASK");
}
closedir(d);
printf("%s\n", hit ? " <-- HIT" : "");
return;
}
fd = open(target, O_RDONLY);
if (fd < 0) {
printf("container: %s: open(%s) = %s\n", when, target, strerror(errno));
return;
}
if (read(fd, buf, sizeof(buf) - 1) < 0)
die("read");
close(fd);
if (strchr(buf, '\n'))
*strchr(buf, '\n') = 0;
printf("container: %s: read(%s) = \"%s\"%s\n", when, target, buf,
strncmp(buf, "MASK", 4) ? " <-- HIT" : "");
}
int main(int argc, char **argv)
{
const char *name = argc > 1 ? argv[1] : "net/dev";
const char *op = argc > 2 ? argv[2] : "stat";
int isdir = !strcmp(name, "net/stat");
int to_host[2], to_cont[2];
struct stat st;
pid_t pid;
char c;
setvbuf(stdout, NULL, _IOLBF, 0);
snprintf(target, sizeof(target), "/proc/%d/%s", getpid(), name);
if (unshare(CLONE_NEWNS) || mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL))
die("unshare");
mkdir(WORK, 0755);
if (mount("W", WORK, "tmpfs", 0, NULL))
die("mount W");
write_str(WORK "/mask", "MASK", O_CREAT);
mkdir(WORK "/maskdir", 0755);
write_str(WORK "/maskdir/MASK", "MASK", O_CREAT);
if (pipe(to_host) || pipe(to_cont))
die("pipe");
pid = fork();
if (pid < 0)
die("fork");
if (!pid) {
if (unshare(CLONE_NEWNS))
die("container unshare");
if (mount(isdir ? WORK "/maskdir" : WORK "/mask", target, NULL, MS_BIND, NULL)) {
printf("container: bind the mask on %s: %s\n", target, strerror(errno));
exit(0);
}
printf("container: bind the mask on %s: ok\n", target);
if (setgid(1000) || setuid(1000) || prctl(PR_SET_DUMPABLE, 1))
die("setuid");
/* lock the mount: a new user namespace with a copy of the tree */
if (unshare(CLONE_NEWUSER | CLONE_NEWNS))
die("container userns");
write_str("/proc/self/setgroups", "deny", 0);
write_str("/proc/self/uid_map", "0 1000 1", 0);
write_str("/proc/self/gid_map", "0 1000 1", 0);
mounts("before");
errno = 0;
if (!strcmp(op, "stat")) {
stat(target, &st);
printf("container: stat(%s): %s\n", target, strerror(errno));
} else if (!strcmp(op, "umount")) {
umount2(target, MNT_DETACH);
printf("container: umount2(%s, MNT_DETACH): %s\n", target, strerror(errno));
} else if (write(to_host[1], "g", 1) != 1 || read(to_cont[0], &c, 1) != 1) {
die("sync");
}
mounts("after");
show("after", isdir);
exit(0);
}
close(to_host[1]);
if (read(to_host[0], &c, 1) == 1) {
pid_t p2 = fork();
if (!p2) {
if (setgid(65534) || setuid(65534))
die("setuid");
errno = 0;
stat(target, &st);
printf("nobody: uid %u: stat(%s): %s\n", getuid(), target, strerror(errno));
exit(0);
}
waitpid(p2, NULL, 0);
if (write(to_cont[1], "g", 1) != 1)
die("sync");
}
waitpid(pid, NULL, 0);
return 0;
}
Fixes: 1fde6f21d90f ("proc: fix /proc/net/* after setns(2)")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261024-work-procfs-fixes-v1-1-46b03c3a8fac@kernel.org
Acked-by: Jan Kara <jack@suse.cz>
Reviewed-by: Aleksa Sarai <aleksa@amutable.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
current_hweight() publishes the cache generation and both weights with
separate stores. Refreshers running on different CPUs can interleave
their stores and leave a new generation paired with weights from an older
calculation. The compiler can also reorder the generation store before
the weight stores, so a reader that sees the new generation can accept
stale weights.
A stale weight is not only a reporting artifact. It feeds the cost
calculation and the donation logic, where an inconsistent combination can
lead to a zero divisor in transfer_surpluses().
In addition, the slow path rereads iocg->hweight_active/inuse after
publishing them, so the value it returns can be the one written by another
refresher instead of the locally computed one.
Pack the generation and both weights into a single atomic64_t and publish
them with one atomic write. Readers take one atomic snapshot and return
the local values. Weights are in [1, WEIGHT_ONE]; storing each minus one
preserves the full range in 16 bits and leaves 32 bits for the generation.
The two weight fields remain as the donation workspace used under
ioc->lock. Tracepoints decode the packed cache without refreshing it and
iocost_monitor.py is updated accordingly.
Developed with assistance from Claude Code (Anthropic): the assistant
drafted the packed representation, the helpers and the tracepoint/monitor
changes, and ran the checks listed below. The author reviewed the result
and remains responsible for it.
Fixes: 7caa47151ab2 ("blkcg: implement blk-iocost")
Signed-off-by: Kunhai Dai <daikunhai@didiglobal.com>
Tested-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Tao Cui <cuitao@kylinos.cn>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://patch.msgid.link/20261019125412.3123895-3-daikunhai@didiglobal.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
transfer_surpluses() warns and then keeps going when the donation
calculation produced invalid weights. The following propagation can then
divide by a zero hweight_inuse, so the warning is immediately followed by a
divide error.
When the check trips, abandon this period's donation transfer and release
the walk lists. This trades one period without donation for not crashing;
the invalid state is not expected on the normal path.
Developed with assistance from Claude Code (Anthropic): the assistant
analysed the failure path, drafted the change and ran the checks listed
below. The author reviewed the result and remains responsible for it.
Fixes: e08d02aa5fc9 ("blk-iocost: implement Andy's method for donation weight updates")
Cc: stable@vger.kernel.org
Signed-off-by: Kunhai Dai <daikunhai@didiglobal.com>
Tested-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Tao Cui <cuitao@kylinos.cn>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://patch.msgid.link/20261019125412.3123895-2-daikunhai@didiglobal.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
Christian Brauner <brauner@kernel.org> says:
The exec securebits are meant to be set and locked by a service manager
so that nothing below it can turn them off again. But creating a user
namespace resets all securebits, locks included. So anyone who may
create a user namespace can shed them with a single
unshare(CLONE_NEWUSER), and joining one with setns() does the same.
That's pretty much everyone.
systemd gains a SecureBits= setting for the service manager itself. It
sets and locks the exec bits for everything it starts, so interpreters
that support AT_EXECVE_CHECK enforce the system's execution policy for
their scripts. With the reset "unshare -U python3 /tmp/x.py" gets
around all of that.
The other securebits are about how capabilities are computed and
resetting them in a new user namespace is fine. The exec bits grant
nothing. They're restrictions a task placed on itself, like
no_new_privs, which a user namespace doesn't drop either. So keep them
and their locks and keep resetting the rest.
I also keep the exec bits that aren't locked. fork() and execve()
preserve them and the task can clear them itself anyway, so dropping
them on unshare() would just be inconsistent.
The new selftests fail on mainline and pass with the fix. I think this
wants to go to stable, otherwise the locks don't mean much on any kernel
that has these bits.
* patches from https://patch.msgid.link/20261026-work-securebits-userns-v1-0-b35473fa1a34@kernel.org:
selftests/exec: check that user namespaces keep the exec securebits
userns: don't drop the exec securebits
Link: https://patch.msgid.link/20261026-work-securebits-userns-v1-0-b35473fa1a34@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Check that the exec securebits survive creating a user namespace with
unshare(CLONE_NEWUSER) and joining one with setns():
- both bits with their locks, which can't be changed in there
- both bits without their locks, which can still be cleared in there
- only the locks, which keep the bits from being set in there
The other securebits are reset as before.
Link: https://patch.msgid.link/20261026-work-securebits-userns-v1-2-b35473fa1a34@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE tell script
interpreters to only run what AT_EXECVE_CHECK allows and to refuse
interactive input. A service manager sets them together with their lock
bits so that nothing below it can turn them off again.
But set_cred_user_ns() resets all securebits to SECUREBITS_DEFAULT. So
any task that can create a user namespace sheds the locked exec bits
with a single unshare(CLONE_NEWUSER) and joining one with setns() does
the same. The lock is pointless if anyone who may create a user
namespace can drop it, and that is pretty much everyone.
Resetting the other bits is fine. They are about how capabilities are
computed and a new user namespace starts out with a full set of
capabilities anyway. The exec bits grant nothing though. They are
restrictions a task placed on itself, just like no_new_privs which a
user namespace doesn't touch either. So keep them and their locks.
Reproducer:
// SPDX-License-Identifier: GPL-2.0
/* Lock the exec securebits, then create a user namespace and look at them again. */
#define _GNU_SOURCE
#include <sched.h>
#include <stdio.h>
#include <sys/prctl.h>
#define SECBIT_EXEC_RESTRICT_FILE (1 << 8)
#define SECBIT_EXEC_RESTRICT_FILE_LOCKED (1 << 9)
#define SECBIT_EXEC_DENY_INTERACTIVE (1 << 10)
#define SECBIT_EXEC_DENY_INTERACTIVE_LOCKED (1 << 11)
int main(void)
{
int bits = SECBIT_EXEC_RESTRICT_FILE | SECBIT_EXEC_RESTRICT_FILE_LOCKED |
SECBIT_EXEC_DENY_INTERACTIVE | SECBIT_EXEC_DENY_INTERACTIVE_LOCKED;
if (prctl(PR_SET_SECUREBITS, bits, 0, 0, 0)) {
perror("PR_SET_SECUREBITS");
return 1;
}
printf("before: 0x%x\n", prctl(PR_GET_SECUREBITS, 0, 0, 0, 0));
printf("clear locked bits: %s\n", prctl(PR_SET_SECUREBITS, 0, 0, 0, 0) ? "refused" : "ALLOWED");
if (unshare(CLONE_NEWUSER)) {
perror("unshare(CLONE_NEWUSER)");
return 1;
}
printf("after unshare(CLONE_NEWUSER): 0x%x\n", prctl(PR_GET_SECUREBITS, 0, 0, 0, 0));
return 0;
}
On v7.1 this prints "before: 0xf00", "clear locked bits: refused" and
"after unshare(CLONE_NEWUSER): 0x0". With this patch the last line
reads 0xf00 as well.
Fixes: a0623b2a1d59 ("security: Add EXEC_RESTRICT_FILE and EXEC_DENY_INTERACTIVE securebits")
Cc: stable@vger.kernel.org # v6.14+
Link: https://patch.msgid.link/20261026-work-securebits-userns-v1-1-b35473fa1a34@kernel.org
Reviewed-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
rnbd_sess_dev_alloc() stores the empty sess_dev in the XArray as its
first step. An IO message with the new ID from another connection can
find it before bdev_file is set, or hold a reference while a failing
OPEN frees it after a mere RCU grace period. Both are reachable from any
host that can open a device.
Reserve the ID with a NULL entry and store the sess_dev once nothing can
fail anymore. The error path still erases the ID, and no longer needs
synchronize_rcu().
Fixes: 2de6c8de192b ("block/rnbd: server: main functionality")
Cc: "Md. Haris Iqbal" <haris.iqbal@ionos.com>
Cc: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Chris Mason <mason@kernel.org>
Link: https://patch.msgid.link/20261028164415.1262013-41-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
process_msg_close() looks the sess_dev up with a kref, drops it right
away, and only then takes the session mutex for the teardown. A session
has several connections handled by different workers, so two CLOSE
messages for the same ID both finish the lookup before either one tears
the sess_dev down, and the second one works on freed memory. Any host
that can open a device on the server can do that.
Take the mutex first and look the ID up with xa_load() under it. The
second CLOSE then finds nothing.
Fixes: 2de6c8de192b ("block/rnbd: server: main functionality")
Cc: "Md. Haris Iqbal" <haris.iqbal@ionos.com>
Cc: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Chris Mason <mason@kernel.org>
Link: https://patch.msgid.link/20261028164415.1262013-40-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
rnbd_srv_create_dev_session_sysfs() drops the kobject reference when it
fails. The release function is rnbd_destroy_sess_dev(), which takes
srv_dev->lock, and the caller holds that lock: the worker deadlocks on
itself. A client opening one device as "ram0" and "./ram0" gets there
through -EEXIST from kobject_add().
Have process_msg_open() drop the reference after releasing the lock, and
leave the rest of the cleanup to the release function.
Fixes: 8cee532f469b ("block/rnbd: server: sysfs interface functions")
Fixes: d3a95ccaaf4d ("block/rnbd: call kobject_put in the failure path")
Cc: "Md. Haris Iqbal" <haris.iqbal@ionos.com>
Cc: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261028164415.1262013-18-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
rnbd_srv_create_dev_sysfs() drops the only kobject reference on its
error paths, and the release function frees the rnbd_srv_dev.
process_msg_open() keeps using it and ends in destroy_device() with a
list_del() and a second kfree(). This doesn't need memory pressure: a
client opening a device while its last mapping goes away gets -EEXIST
from kobject_add() against the old directory.
Only undo kobject_add() on failure and leave the reference to
destroy_device(). Initialise the kobject once, the next open retries.
Fixes: 8cee532f469b ("block/rnbd: server: sysfs interface functions")
Fixes: d3a95ccaaf4d ("block/rnbd: call kobject_put in the failure path")
Cc: "Md. Haris Iqbal" <haris.iqbal@ionos.com>
Cc: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261028164415.1262013-17-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
_drbd_set_state() must be called with req_lock held. drbd_adm_new_c_uuid()
does, receive_uuids() on the peer doesn't, so a concurrent state change
gets lost: the device is Secondary again, or its failed disk UpToDate,
while the work for the lost change is already queued.
Fixes: b411b3637fa7 ("The DRBD driver")
Cc: Philipp Reisner <philipp.reisner@linbit.com>
Cc: Lars Ellenberg <lars.ellenberg@linbit.com>
Cc: "Christoph Böhmwalder" <christoph.boehmwalder@linbit.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261028164415.1262013-15-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
start_resync_timer gets armed for up to a second and nothing ever
deletes it, so drbd_destroy_device() can free the device with the timer
pending. Shut it down where request_timer is shut down.
Fixes: 370a43e7982d ("drbd: Work on the Ahead -> SyncSource transition")
Cc: Philipp Reisner <philipp.reisner@linbit.com>
Cc: Lars Ellenberg <lars.ellenberg@linbit.com>
Cc: "Christoph Böhmwalder" <christoph.boehmwalder@linbit.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261028164415.1262013-13-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
The integrity buffer is a single segment and must not exceed
max_segment_size. max_integrity_io_size() applies that limit to the data
size instead, which is off by a factor of 64 for 8 bytes of PI per 512
bytes of data. A SCSI host that doesn't set a maximum segment size gets
64KiB, lpfc and qla2xxx among them, so a DIX disk behind one of those
does no IO above 64KiB.
Limit the buffer and derive the data size from that, which makes it 4MiB
for that case.
Fixes: ec7f31b2a2d3 ("block: make bio auto-integrity deadlock safe")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261028164415.1262013-45-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
Commit ad0d05dbddc1 keeps struct blk_mq_tags and the requests until the
iterators under tags_srcu are gone, but the two bitmaps they walk are
still freed right away. When the number of hardware queues shrinks,
blk_mq_tagset_busy_iter() may be about to walk tags that
__blk_mq_update_nr_hw_queues() frees. sbitmap_free() clears the map
pointer, so that's a NULL dereference or a use-after-free, depending on
the timing.
Free the bitmaps with the rest in the callback.
Fixes: ad0d05dbddc1 ("blk-mq: Defer freeing of tags page_list to SRCU callback")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Link: https://patch.msgid.link/20261028164415.1262013-44-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
start_block and block_count are converted to 512 byte sectors in 32-bit
arithmetic, although put_partition() takes a sector_t. With a block size
above 512 bytes anything beyond 2TiB wraps and the partition lands on
top of another one.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261028164415.1262013-43-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
blk_mq_init_allocated_queue() only takes tag_list_lock at the end, when
it adds the queue to the set. Before that it sizes the queue by
set->nr_hw_queues, hands out set->tags[], maps by set->map[], and
blk_mq_map_swqueue() frees tags of the set. An update clearing the maps
at that time makes the new queue free tags that all the other queues
share. An NVMe controller reset during a namespace scan does exactly
that.
Take the lock before the first look at the set. An update can be part of
error handling, so the allocations under the lock must not wait for IO:
memalloc_noio_save(), as in add_disk_fwnode().
Fixes: 868f2f0b7206 ("blk-mq: dynamic h/w context count")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261028164415.1262013-39-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
blk_mq_update_nr_hw_queues() exits and replaces the hctxs of every queue
on set->tag_list under tag_list_lock. blk_mq_exit_queue() exits its
hctxs without the lock and only then takes the queue off the list, so an
update running at the same time exits them again and sets up new ones
that nobody exits. del_gendisk() is kept away by update_nr_hwq_lock,
disk_release() of a never added disk and blk_mq_destroy_queue() are not,
and nvme_alloc_ns() drops such a disk when it finds the queues frozen by
a reset.
Hold tag_list_lock for both steps.
Fixes: 630ef623ed26 ("blk-mq: Swap two calls in blk_mq_exit_queue()")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261028164415.1262013-38-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
blk_mq_quiesce_tagset() only quiesces the queues on the list at that
moment. A queue added before blk_mq_unquiesce_tagset() runs unquiesced,
and then gets unquiesced without ever having been quiesced, which trips
the warning in blk_mq_unquiesce_queue(). NVMe gets there when a
controller fails while its namespaces are being scanned.
Count the quiesce calls in the tag set and quiesce a joining queue that
many times, under a spinlock that also serializes the two functions. It
can't be tag_list_lock, see commit 59e25ef2b413.
Fixes: 414dd48e882c ("blk-mq: add tagset quiesce interface")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261028164415.1262013-37-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
blk_rq_dma_map_iter_start() promises that the caller need not initialize
the dma_iova_state, but the P2P bus address path returns before it does
so. The state then holds what the last request with that tag left, and
blk_rq_dma_map_coalesce() describes a transfer as one IOVA range that it
isn't. Clear it on that path too.
Fixes: 858299dc6160 ("block: add scatterlist-less DMA mapping helpers")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261028164415.1262013-33-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
If ->timeout() returns BLK_EH_RESET_TIMER, blk_add_timer() gives the
request a new deadline that blk_mq_timeout_work() doesn't know about
when it picks the next expiry. A 2s nbd read next to a 60s one expires
once and then not again for a minute. Let the new deadline take part.
Fixes: 320ae51feed5 ("blk-mq: new multi-queue block IO queueing mechanism")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261028164415.1262013-32-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
The timeout handler sets and clears RQF_TIMED_OUT in a request it only
holds a reference to, with a plain read-modify-write, while the
completion clears flags in rq_flags the same way. If the two interleave,
blk_mq_finish_request() runs twice with the flags back: the zone write
plug reference is dropped twice and the scheduler's ->finish_request()
is called again.
Change RQF_TIMED_OUT with cmpxchg, so the handler never writes back a
flag that changed under it.
Fixes: e5c0ca13659e ("blk-mq: release scheduler resource when request completes")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261028164415.1262013-31-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
A hctx only gets BLK_MQ_F_TAG_QUEUE_SHARED from
blk_mq_add_queue_tag_set(). The hctxs blk_mq_update_nr_hw_queues()
allocates for queues that joined long ago never get it, so they aren't
counted in active_queues and hctx_may_queue() lets them take all the
tags. One NVMe namespace can starve the others after the first update.
Set the flag on the new hctxs.
Fixes: 868f2f0b7206 ("blk-mq: dynamic h/w context count")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261028164415.1262013-30-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
blk_mq_poll() indexes queue_hw_ctx[] with bio->bi_cookie unchecked. Zone
write plugging keeps the segment count of a plugged write in
__bi_nr_segments, which shares a union with the cookie, and clears
REQ_POLLED, but bio_poll() only looks at the cookie. A plugged polled
write with more segments than there are hardware queues reads past the
array. The bio is also SLAB_TYPESAFE_BY_RCU, so a cookie from a bio
reused for another device can meet the wrong array as well.
Ignore a cookie that is out of range or doesn't name a poll queue.
Fixes: 3e08773c3841 ("block: switch polling to be bio based")
Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261028164415.1262013-28-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
flush_end_io() stores the status in fq->rq_status for whoever drops the
last reference to finish the flush sequence. blk_mq_put_rq_ref() calls
it with BLK_STS_OK from tag iterators and the timeout handler and
overwrites an error the real completion stored. With two extra
references, reading /sys/block/<disk>/inflight is enough for that, a
failed flush completes everything waiting on it with success and fsync()
lies to the application.
Only store a status that is an error.
Fixes: 2e315dc07df0 ("blk-mq: grab rq->refcount before calling ->fn in blk_mq_tagset_busy_iter")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261028164415.1262013-6-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
sync_bdevs() holds a reference to the bdev inode while it takes
bd_disk->open_mutex. For a partition the inode doesn't keep the gendisk
alive, the device reference does: part_release() drops the disk as soon
as the last reference to the partition device is gone. A sync(2) racing
with the last close of a partition of an already deleted disk locks a
freed open_mutex.
Take a device reference before touching the disk, like
blkdev_get_no_open() does, and skip the bdev if it's on its way out.
Fixes: a8698707a183 ("block: move bd_mutex to struct gendisk")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261028164415.1262013-5-axboe@kernel.dk
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
mntns_install() refuses to switch the mount namespace of a task that
shares its fs_struct, since root and cwd of the others would move to a
namespace they are not in. It looks at nsset->fs for that. If more
than the mount namespace is asked for, that is a private copy made by
prepare_nsset(), which has one user by definition; commit_nsset() then
writes root and cwd of the copy to the fs_struct of the task.
A thread of a process with two threads, pidfd of a task in another
mount namespace:
setns(pidfd, CLONE_NEWNS) -EINVAL
setns(pidfd, CLONE_NEWNS | CLONE_NEWUTS) 0
and after the second call the other thread, still in the old mount
namespace, has lost its cwd (/tmp became /) and sees the mounts of the
other namespace under /.
Look at the fs_struct that is going to be changed. The second call then
fails with -EINVAL like the first one.
Fixes: 303cc571d107 ("nsproxy: attach to namespaces via pidfds")
Cc: stable@vger.kernel.org # v5.8+
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
lookup_inode_permission_may_exec() asserts that it is only ever handed a
directory, on the grounds that link_path_walk() has already checked
d_can_lookup(). In rcu-walk that does not follow. step_into() samples
->d_inode and validates it against ->d_seq, but link_path_walk() loads
->d_flags again afterwards, and nothing orders that load against changes
of the dentry:
CPU0 (rcu-walk of "x/y") CPU1
step_into(x)
inode = x->d_inode // regular file
read_seqcount_retry(&x->d_seq) == 0
nd->inode = inode
unlink("x")
d_delete() // x goes negative
mkdir("x")
d_instantiate() // same dentry
d_can_lookup(x) // true now
may_lookup()
VFS_BUG_ON_INODE(!S_ISDIR(nd->inode->i_mode))
With CONFIG_PREEMPT_RCU the walker can be preempted in that window, so it
is not limited to a few instructions. mkdir() only gets the same dentry
back if the filesystem keeps negative dentries around. tmpfs doesn't,
ext4 does.
Apart from the assertion the stale inode is harmless, and has been there
since before the assertion was added: nd->seq no longer matches x->d_seq,
so lookup_fast(), follow_dotdot_rcu() and try_to_unlazy() all fail with
-ECHILD and the walk is redone in ref-walk, whatever the permission check
on the old inode said.
Keep the assertion for ref-walk, where the dentry is pinned and can't
change type, and skip it for rcu-walk. No change for kernels without
CONFIG_DEBUG_VFS.
Fixes: e631df89cd5d ("fs: speed up path lookup with cheaper handling of MAY_EXEC")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Christian Brauner <brauner@kernel.org> says:
Two iomap fixes that came out of Josef's AI bug scans.
The first one is old. For a buffered write into a hole, an unwritten
extent or past EOF __iomap_write_begin() marks the zeroed blocks
uptodate before the copy from userspace. If the copy comes up short the
uncopied part of the write stays behind in an uptodate block:
ftruncate(fd, 4096);
pwrite(fd, p, 3000, 100); /* 16 bytes at p, then PROT_NONE */
pread(fd, buf, 4096, 0);
That returns 2984 bytes of whatever was in the page before. Willy
reported this in 2024 and Zhang Yi posted a fix but it never got merged.
I went with the same fix. Josef's version zeroed the partially written
blocks completely and only marked those uptodate. That works too but
it's a lot more code for the same result.
Willy suggested back then to drop the uptodate marking from
__iomap_write_begin() completely. I didn't do that. Blocks that were
read from disk are valid and keeping them uptodate means a retried short
copy doesn't have to read them again.
The second one is from v7.1. When the filesystem handles the integrity
metadata the bio size check wraps if a single chunk is larger than
max_integrity_io_size(). That limit is capped by the maximum segment
size, so it's 64 KiB on lpfc and qla2xxx, and XFS writeback of a large
preallocated file happily builds a single 300 MiB bio and overruns the
2 MiB integrity buffer.
Note that a single chunk above the limit still gets a bio of its own.
That's fine with 4k pages. But with 64k pages a folio can be 128 MiB
which needs 4 MiB of PI with 16 bytes of metadata per 512 byte interval.
That's not new though and needs a separate fix.
Both are tested in a VM. Without the first patch the short copy
reproducer leaks on XFS with 4k and 1k blocks and on ext4 with buffered
iomap, and an fsx-like checker with faulting user buffers fails within
a few hundred operations. With it both pass. The second patch was tested
with scsi_debug DIF/DIX patched to a 64 KiB maximum segment size.
* patches from https://patch.msgid.link/20261029-work-iomap-ai-fixes-v1-0-7eda63bc1b13@kernel.org:
iomap: keep large chunks from defeating the bio cap
iomap: don't mark unwritten parts of a folio uptodate
Link: https://patch.msgid.link/20261029-work-iomap-ai-fixes-v1-0-7eda63bc1b13@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
For integrity I/O a bio must not grow beyond max_integrity_io_size()
because the integrity buffer is a single allocation of at most 2 MiB.
Writeback and buffered reads check that before adding to a bio:
bio->bi_iter.bi_size > iomap_max_bio_size(iomap) - len
The subtraction is done in size_t and wraps if len alone is above the
limit. The check then never fires and the bio grows until it runs out
of bio_vecs.
That's easy to hit. The limit is also capped by the maximum segment size
which is 64 KiB for SCSI hosts that don't set it, such as lpfc and
qla2xxx, and XFS uses folios of up to 2 MiB. Writeback of a preallocated
300 MiB file then builds a single bio that needs 4.7 MiB of PI with 512
byte sectors and 8 byte tuples. That's more than kmalloc() can do, so
bio_integrity_alloc_buf() falls back to a 2 MiB mempool element and
generating the PI writes 2.7 MiB past its end.
Check the length on its own first. A chunk above the limit still gets a
bio of its own which is never larger than a folio.
Fixes: 0b10a370529c ("iomap: support T10 protection information")
Cc: stable@vger.kernel.org # v7.1+
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261029-work-iomap-ai-fixes-v1-2-7eda63bc1b13@kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
For a buffered write into a hole, an unwritten or new extent, or past
EOF __iomap_write_begin() zeroes the parts of the blocks outside of the
write and then marks the blocks uptodate. That includes the range of the
write itself which still holds whatever was in the folio before. It only
becomes valid once the copy from userspace has filled it in.
But the copy can come up short and __iomap_write_end() only rejects a
short copy if the folio isn't uptodate. If the zeroed blocks were all
that was missing the folio is uptodate by then, the short copy is
accepted, and the rest of the range is exposed to readers and can end up
on disk. A source buffer that ends in an unmapped page is enough:
ftruncate(fd, 4096);
pwrite(fd, p, 3000, 100); /* 16 bytes at p, then PROT_NONE */
pread(fd, buf, 4096, 0);
The write returns 16 and the read returns 2984 bytes of whatever was in
that page before. If other blocks of the folio aren't uptodate the short
copy is rejected but the zeroed blocks keep their uptodate bits and the
retry with the copied length leaves the same bytes behind.
Only mark blocks uptodate in __iomap_write_begin() after reading them.
__iomap_write_end() marks the zeroed blocks uptodate once the copy has
succeeded and rejects a short copy.
Willy reported this back in 2024 (cf. [1]) and Zhang Yi posted the same
fix (cf. [2]) but it never got merged.
[1]: https://lore.kernel.org/all/ZqsN5ouQTEc1KAzV@casper.infradead.org
[2]: https://lore.kernel.org/all/20240812121159.3775074-6-yi.zhang@huaweicloud.com
Fixes: 9dc55f1389f9 ("iomap: add support for sub-pagesize buffered I/O without buffer heads")
Reported-by: Josef Bacik <josef@toxicpanda.com>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261029-work-iomap-ai-fixes-v1-1-7eda63bc1b13@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Commit 8db6c34f1dbc ("Introduce v3 namespaced file capabilities") changed
cap_inode_setxattr() to return 0 unconditionally for XATTR_NAME_CAPS
("security.capability") and moved capability validation into
cap_convert_nscap(), but placed the call inside the "if (size)" block in
setxattr(). Commit 7c03e2cda4a5 ("vfs: move cap_convert_nscap() call into
vfs_setxattr()") preserved that "size &&" condition in vfs_setxattr():
if (size && strcmp(name, XATTR_NAME_CAPS) == 0) {
error = cap_convert_nscap(idmap, dentry, &value, size);
...
}
For security.* extended attributes, xattr_permission() skips
inode_permission(MAY_WRITE) and leaves permission checks to the security
module, while cap_inode_setxattr() returns 0 for XATTR_NAME_CAPS assuming
cap_convert_nscap() has already validated the write. Because
__vfs_setxattr() treats size == 0 as an empty attribute write (setting
value = "") rather than a removal, calling setxattr(2) on
"security.capability" with size == 0 bypasses cap_convert_nscap() and
allows an unprivileged caller without CAP_SETFCAP or write permission on
the target inode to write a 0-byte security.capability xattr (or
overwrite an existing one).
When that binary is subsequently executed, get_vfs_caps_from_disk() reads
the 0-byte security.capability xattr and fails the
size < sizeof(magic_etc) check with -EINVAL, causing execve(2) to fail
with -EINVAL.
Always invoke cap_convert_nscap() when strcmp(name, XATTR_NAME_CAPS) == 0
regardless of size, so that 0-byte writes hit the existing !*ivalue /
!validheader() checks in cap_convert_nscap() and are rejected with
-EINVAL.
Fixes: 8db6c34f1dbc ("Introduce v3 namespaced file capabilities")
Signed-off-by: Dominik Maier <dmnk@google.com>
Link: https://patch.msgid.link/20261022232119.2537209-1-dmnk@google.com
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Commit 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
bypassed `tg3_power_down()` on system restarts to prevent reboot hangs and
NMIs on servers where NICs were initialized via SNP (such as Dell PowerEdge
R650 and R750).
However, skipping the power-down flow globally causes the ACPI _PTS method
to trigger fatal PCIe AER events and hangs on other models. Commit
e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot") attempted to
mitigate this by adding an AER-disable DMI quirk table. Maintaining a
growing list of models for a secondary symptom misses the root cause,
leaving systems like the PowerEdge T440 (sharing the BCM5720 NIC) exposed.
Fix this by inverting the quirk logic:
1. Restrict SNP power-down avoidance strictly to
`tg3_restart_snp_quirk_table` for the specific systems requiring it
(Dell PowerEdge R650 and R750).
2. Restore standard power-down behavior for all other systems, eliminating
the separate AER-disable quirk table.
For systems affected by the original SNP restart issue, their shutdown flow
remains completely untouched.
Link: https://lore.kernel.org/netdev/20251221175050.14089-1-noam.d.eliyahu@gmail.com/T/#u
Fixes: 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
Fixes: e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot")
Cc: stable@vger.kernel.org
Signed-off-by: Noam D. Eliyahu <noam.d.eliyahu@gmail.com>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Link: https://patch.msgid.link/20261026220955.473247-1-noam.d.eliyahu@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The interface can continue reporting carrier up after the link goes
down because the driver ignores the firmware's link-down notification.
Firmware reports link-down events with a zero port_magic and no
valid speed or duplex information. The driver requires ST_VALID_MAGIC
for every link event and returns before updating the cached link state
when this check fails. It also writes an invalid driver snapshot to
firmware instead of acknowledging the link-down state.
Validate the speed magic only for link-up events. This allows link-down
events to update the cached state, propagate carrier loss, and update
the snapshot exported to firmware. Keep the checks for invalid link-up
magic and unsupported speeds.
Fixes: 92a671d510a8 ("net: rnpgbe: Add link status handling support")
Signed-off-by: Dong Yibo <dong100@mucse.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/C2DFBBF04F1DCFD2+20261026075445.920572-1-dong100@mucse.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
r8152 detects carrier from PLA_PHYSTATUS without reading BMSR, so
BMSR_LSTATUS can remain latched low after link up. Since
commit f6f2e946aa4d ("net: mii: Fix the Speed display when the network
cable is not connected"), the first speed query can then report
SPEED_UNKNOWN, which applications such as NetworkManager may cache
until the next carrier change.
Use BMSR_LSTATUS in set_carrier() so a latched link down can turn
carrier off after the link has recovered. If BMSR reports down but
PLA_PHYSTATUS reports up, reschedule the link work to read the current
state without waiting for another notification.
Tested on an RTL8153B: NetworkManager reported 1000 Mb/s after 3 of 3
rebinds, instead of an unknown speed.
Fixes: f6f2e946aa4d ("net: mii: Fix the Speed display when the network cable is not connected")
Cc: stable@vger.kernel.org
Suggested-by: Hayes Wang <hayeswang@realtek.com>
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
Reviewed-by: Hayes Wang <hayeswang@realtek.com>
Link: https://patch.msgid.link/20261026012345.1902285-1-lilinmao@kylinos.cn
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
otx2_dma_map_skb_frag() calls skb_unclone_full() on its own copy of the
skb pointer. For a cloned skb, for example with tcpdump running on the
interface, this frees the skb and the new copy is lost on return. The
transmit path then uses the freed skb and the copy leaks.
Make the skb writable in place in otx2_sq_append_skb() instead, so the
pointer stays valid. Also do this for nonlinear skbs, as the hardware
writes the encrypted data back into the frags, which may be shared,
for example with the TCP retransmit queue.
Fixes: 195c3d463181 ("octeontx2-pf: map skb data as device writeable")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Link: https://patch.msgid.link/20261027-nb-otx2-xfrm-unshare-net-v3-1-31dc9d79d11d@tipi-net.de
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
dup_task_struct() sets rcu_users to 2. From then on bpf_task_acquire()
hands out references to the new task, which BPF programs see as a trusted
pointer long before it is sure to live: in the task_alloc LSM hook and in
ops.init_task() of sched_ext.
If copy_process() fails later on, it undoes what it has done and frees
the task without a look at the count. A program that has stored its
reference in a map is left with a pointer to freed memory:
BUG: KASAN: slab-use-after-free in put_task_struct_rcu_user+0x22/0x90
The program is loaded by the administrator, but any user can make a fork
fail, for example at the pids.max of a cgroup.
Honouring the count would not do: the task would stay behind with its mm,
files and credentials freed. Leave rcu_users at zero until copy_process()
cannot fail any more. bpf_task_acquire() returns NULL until then, which
its callers have to be prepared for, and which they get for init_task,
whose count has always been zero. The other users of rcu_users deal with
tasks that have run.
Fixes: 90660309b0c7 ("bpf: Add kfuncs for storing struct task_struct * as a kptr")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Nicolai Buchwitz says:
====================
octeontx2-pf: fix ipsec transmit error paths
Two pre-existing bugs in the ipsec transmit error paths, flagged by the
clashiko review of my earlier ipsec fix for this driver [1].
Both look like edge cases to me, but at least the path in patch 1 (an SA
removed during rekey or teardown while packets are still queued) looks
plausible to me. But don't take my word for it, I'm not an octeontx2 user.
Compile-tested only, I have no octeontx2 hardware.
[1] https://lore.kernel.org/all/20261022-nb-otx2-xfrm-unshare-net-v2-1-5590c9325bcd@tipi-net.de/
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
====================
Link: https://patch.msgid.link/20261027-nb-otx2-ipsec-tx-fixes-net-v1-0-2114c4f10c36@tipi-net.de
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
After a DMA mapping error, the unmap reads an skb pointer that is not
set yet. It can be NULL or left over from an earlier packet.
Fixes: 195c3d463181 ("octeontx2-pf: map skb data as device writeable")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Acked-by: Ratheesh Kannoth <rkannoth@marvell.com>
Link: https://patch.msgid.link/20261027-nb-otx2-ipsec-tx-fixes-net-v1-2-2114c4f10c36@tipi-net.de
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
When an SA is removed while packets for it are still queued, the
transmit path frees the packet but reports the queue as busy. The stack
then requeues the freed skb, and the DMA mappings leak.
Fixes: 6a77a158848a ("cn10k-ipsec: Process outbound ipsec crypto offload")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Ratheesh Kannoth <rkannoth@marvell.com>
Link: https://patch.msgid.link/20261027-nb-otx2-ipsec-tx-fixes-net-v1-1-2114c4f10c36@tipi-net.de
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Jamal Hadi Salim says:
====================
net/sched: sch_teql: fix transmit-path regressions
- Patch 1/2 is unchanged
- Patch 2/2 now fixes the drop accounting raised by the Sashiko
review of v1 (both providers): only NETDEV_TX_OK and NET_XMIT_CN
count as accepted transmits (CN can accompany acceptance of the
current packet); any other consumed result, e.g. NET_XMIT_DROP,
is charged to tx_dropped instead of being counted as a good
transmit. The skb-ownership fix is unchanged (Sashiko-gemini).
The v2 revision follows the review of patch 2/2
(https://lore.kernel.org/netdev/179248763469.3135.1739559328868319628@kernel.org/)
and Simon Horman's request for a response
(https://lore.kernel.org/netdev/20261023162821.GN708417@horms.kernel.org/).
Two independent sch_teql regressions on the transmit path.
Patch 1 makes slaves_lock safe for the hard-IRQ-disabled netpoll path and
for PREEMPT_RT by converting it to a raw spinlock taken with IRQs saved.
Patch 2 fixes a use-after-free when a lower virtual device consumes an skb
but returns NET_XMIT_DROP rather than NETDEV_TX_OK, and stops counting
those consumed drops as good transmits.
====================
Link: https://patch.msgid.link/QDISC-Y0BY.v2.r0.20261024060625@mojatatu.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The issue: teql_master_xmit() tests the slave's ndo_start_xmit() return
value against NETDEV_TX_OK alone. A slave that consumes the skb but
returns an error code (rc < NET_XMIT_MASK, e.g. NET_XMIT_DROP) is treated
as if the skb were still owned, so the loop restores skb->dev and calls
__skb_pull() on it. A VLAN slave returns the result of dev_queue_xmit()
from its ndo_start_xmit() (propagated since cbbef5e18307), and when the
lower qdisc drops the packet that value is NET_XMIT_DROP after the skb
has been freed, giving a use-after-free in teql_master_xmit().
The Fix: test for consumption with dev_xmit_complete(), which accepts
NETDEV_TX_OK and every rc < NET_XMIT_MASK, matching other callers that
need to distinguish consumed results from busy. Only NETDEV_TX_OK keeps
the packet counted in tx_packets/tx_bytes and advances the slave; a
consumed result that is neither a success nor NET_XMIT_CN ends the
attempt and is charged to tx_dropped, so lower-qdisc drops are no longer
reported as good transmits. NET_XMIT_CN is counted like success because
it can accompany acceptance of the current packet: pfifo_head_drop drops
the old head, enqueues the current skb and returns NET_XMIT_CN.
Conditions to recreate the bug:
With CONFIG_NET_SCH_TEQL,
CONFIG_VLAN_8021Q and CONFIG_DUMMY enabled, add lkrdummy0 (dummy) and
lkrvlan0 (VLAN id 37 on lkrdummy0); set a pfifo limit 0 root on lkrdummy0
and a teql0 root on lkrvlan0; bring teql0 up with an address and send one
UDP packet to it. The VLAN xmit returns NET_XMIT_DROP from the lower
qdisc while holding a freed skb.
Fixes: 572a9d7b6fc7 ("net: allow to propagate errors through ->ndo_hard_start_xmit()")
Link: https://lore.kernel.org/netdev/179248763469.3135.1739559328868319628@kernel.org/
Link: https://lore.kernel.org/netdev/20261023162821.GN708417@horms.kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: owen kwan <owen@verialabs.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/QDISC-Y0BY.v2.r0.20261024060625-2@mojatatu.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
This is a followup of commit e5b811fe7931 ("net/sched: sch_teql:
Introduce slaves_lock to avoid race condition and UAF") as reported by
Sashiko.
The issue: master->slaves_lock serializes the master's ring of slave
qdiscs. The control paths -- teql_dequeue(), teql_destroy() and
teql_qdisc_init() -- take it with spin_lock_bh(), while
teql_master_xmit() (the master's ndo_start_xmit) takes it with plain
spin_lock().
netpoll_send_skb() wraps the transmit in local_irq_save(), and
__netpoll_send_skb() asserts lockdep_assert_irqs_disabled(), so
teql_master_xmit() reached from netconsole always acquires slaves_lock
with hard IRQs disabled. On PREEMPT_RT spinlock_t is an rtmutex, and
Documentation/locking/locktypes.rst names
local_irq_disable();
spin_lock(&lock);
as a sequence that breaks there because the rtmutex requires a fully
preemptible context. This is reachable without any panic: the nbcon
printer thread calls netconsole's write_thread() callback on every
message. The usual remedy -- folding the IRQ disable into the lock with
spin_lock_irqsave() -- is unavailable because the local_irq_save() is
netpoll's, not ours.
On non-RT the same asymmetry leaves slaves_lock acquirable with hard IRQs
disabled on one path and held with hard IRQs enabled on another. That is
only exploitable during the panic console flush (netconsole is
CON_NBCON_ATOMIC_UNSAFE, so console_is_usable() admits its write_atomic()
only from nbcon_atomic_flush_unsafe(), which panic() alone calls), and
even then an IRQ-saving lock does not help if the holder oopsed inside the
section or was stopped by panic_other_cpus_shutdown(). So the non-RT half
is hardening; the PREEMPT_RT half is the bug.
The Fix: take slaves_lock as a raw spinlock with IRQs saved at every
site. Since skb_queue_purge() takes the queue's spinlock_t and may not
nest inside a raw lock on PREEMPT_RT, move its call in teql_destroy()
outside the raw section; dat->q is the dying qdisc's own queue and was
never serialized by slaves_lock, so the move is behaviour-neutral.
The new critical sections contain only RCU pointer assignments and
netdev_get_tx_queue(); skb_queue_purge(), netif_wake_queue() and
dev_reset_queue() are all outside them. teql_destroy()'s ring walk is
O(slaves), two pointer loads and a compare per iteration, so even a large
ring is microseconds with IRQs off. Keeping the purge conditional rather
than dropping it outright is the conservative call for a backport.
Some context: commit 6301f6a34ed8 ("net/sched: sch_teql: move
rcu_read_lock()/spin_lock() from _bh variants") hardened the parent
fix; together they addressed the master->slaves resurrection
use-after-free and the xmit-side BH warning but left the reverse
direction -- a hard IRQ re-entering xmit while a control path holds the
lock -- unaddressed.
Conditions to recreate the bug:
- CONFIG_PREEMPT_RT, CONFIG_NET_SCH_TEQL, CONFIG_DUMMY and
CONFIG_NETCONSOLE.
- Run `ip link add dev teqlslave0 type dummy`, bring teqlslave0 up, attach
it with `tc qdisc add dev teqlslave0 root teql0`, and bring teql0 up.
- Configure a netconsole target on teql0 with explicit local and remote
IP addresses and a remote MAC address, then emit a console message.
The nbcon printer thread reaches teql_master_xmit() with hard IRQs
disabled by netpoll; taking slaves_lock as a spinlock_t is invalid on
PREEMPT_RT. No panic or concurrent qdisc change is required.
- Reachable by an admin with CAP_NET_ADMIN; teql devices exist only in the
initial netns (no per-netns ops), so a user namespace does not suffice.
Fixes: e5b811fe7931 ("net/sched: sch_teql: Introduce slaves_lock to avoid race condition and UAF")
Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260630150922.238714-1-jhs%40mojatatu.com
Link: https://lore.kernel.org/netdev/20260630150922.238714-1-jhs@mojatatu.com/
Cc: stable@vger.kernel.org
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/QDISC-Y0BY.v2.r0.20261024060625-1@mojatatu.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
When a start-of-message fragment creates a new reassembly key on the
broadcast/bind path, mctp_frag_queue() stores the skb as
key->reasm_head before the key is made observable with mctp_key_add().
If mctp_key_add() fails (a race with another SOM packet for the same
source/dest/tag, or the receiving socket going away), the error path
only drops the key reference via mctp_key_unref(). The final key
destructor frees the key itself but not key->reasm_head, and since the
key was never added to any list, neither the expiry worker nor socket
teardown can reach it to release the queued fragment. The skb is
leaked.
Free key->reasm_head on the key_add failure path before releasing the
key. Other teardown paths (__mctp_key_done_in(), EOM delivery) already
clear reasm_head before the final unref, so this only affects the
failure path.
Fixes: 4a992bbd3650 ("mctp: Implement message fragmentation & reassembly")
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Reported-by: Guikun Yang <yangguikun19@qq.com>
Closes: https://lore.kernel.org/all/tencent_43A65D51143E18518EA968DF9323694EFD08@qq.com/
Acked-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://patch.msgid.link/179287946888.17431.11219154409333681184@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
