On Sat, Nov 02, 2024 at 12:21:32PM +0000, Simon Horman wrote:
> > @@ -2926,16 +2900,18 @@ static int do_recvmmsg(int fd, struct mmsghdr __user *mmsg,
> >
> > datagrams = 0;
> >
> > - sock = sockfd_lookup_light(fd, &err, &fput_needed);
> > - if (!sock)
> > - return err;
> > + CLASS(fd, f)(fd);
> > +
> > + if (fd_empty(f))
> > + return -EBADF;
> > + sock = sock_from_file(fd_file(f));
> > + if (unlikely(!sock))
> > + return -ENOTSOCK;
>
> Hi Al,
>
> There is an unconditional check on err down on line 2977.
> However, with the above change err is now only conditionally
> set before we reach that line. Are you sure that it will always
> be initialised by the time line 2977 is reached?
Nice catch, thank you. It is possible, if you call recvmmsg(2) with
zero vlen and MSG_ERRQUEUE in flags. Which is not going to be done in
any well-behaving code, making it really nasty - nothing like a kernel
bug that shows up only when trying to narrow down a userland bug upstream
of the syscall in question ;-/
AFAICS, that's the only bug of that sort in this commit - all other
places that used to rely upon successful sockfd_lookup_light() zeroing
err have an unconditional assignment to err shortly downstream of that.
Fix folded into commit in question, branch force-pushed; incremental follows
(I would rather not spam the lists with repost of the entire patchset for
the sake of that):
diff --git a/net/socket.c b/net/socket.c
index fb3806a11f94..c3ac02d060c0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2885,7 +2885,7 @@ static int do_recvmmsg(int fd, struct mmsghdr __user *mmsg,
unsigned int vlen, unsigned int flags,
struct timespec64 *timeout)
{
- int err, datagrams;
+ int err = 0, datagrams;
struct socket *sock;
struct mmsghdr __user *entry;
struct compat_mmsghdr __user *compat_entry;
