| Message ID | 206682a8-0604-49e5-8224-fdbe0c12b460@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | watch_queue: fix pipe accounting mismatch | expand |
On Thu, 27 Feb 2025 11:41:08 -0600, Eric Sandeen wrote: > Currently, watch_queue_set_size() modifies the pipe buffers charged to > user->pipe_bufs without updating the pipe->nr_accounted on the pipe > itself, due to the if (!pipe_has_watch_queue()) test in > pipe_resize_ring(). This means that when the pipe is ultimately freed, > we decrement user->pipe_bufs by something other than what than we had > charged to it, potentially leading to an underflow. This in turn can > cause subsequent too_many_pipe_buffers_soft() tests to fail with -EPERM. > > [...] Applied to the vfs-6.15.misc branch of the vfs/vfs.git tree. Patches in the vfs-6.15.misc branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs-6.15.misc [1/1] watch_queue: fix pipe accounting mismatch https://git.kernel.org/vfs/vfs/c/483b7214f602
diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c index 5267adeaa403..41e4e8070923 100644 --- a/kernel/watch_queue.c +++ b/kernel/watch_queue.c @@ -269,6 +269,15 @@ long watch_queue_set_size(struct pipe_inode_info *pipe, unsigned int nr_notes) if (ret < 0) goto error; + /* + * pipe_resize_ring() does not update nr_accounted for watch_queue + * pipes, because the above vastly overprovisions. Set nr_accounted on + * and max_usage this pipe to the number that was actually charged to + * the user above via account_pipe_buffers. + */ + pipe->max_usage = nr_pages; + pipe->nr_accounted = nr_pages; + ret = -ENOMEM; pages = kcalloc(nr_pages, sizeof(struct page *), GFP_KERNEL); if (!pages)
Currently, watch_queue_set_size() modifies the pipe buffers charged to user->pipe_bufs without updating the pipe->nr_accounted on the pipe itself, due to the if (!pipe_has_watch_queue()) test in pipe_resize_ring(). This means that when the pipe is ultimately freed, we decrement user->pipe_bufs by something other than what than we had charged to it, potentially leading to an underflow. This in turn can cause subsequent too_many_pipe_buffers_soft() tests to fail with -EPERM. To remedy this, explicitly account for the pipe usage in watch_queue_set_size() to match the number set via account_pipe_buffers() (It's unclear why watch_queue_set_size() does not update nr_accounted; it may be due to intentional overprovisioning in watch_queue_set_size()?) Fixes: e95aada4cb93d ("pipe: wakeup wr_wait after setting max_usage") Signed-off-by: Eric Sandeen <sandeen@redhat.com ---