On Sun, Nov 03, 2024 at 06:31:13AM +0000, Al Viro wrote:

On Sat, Nov 02, 2024 at 12:21:32PM +0000, Simon Horman wrote:

quoted

quoted

@@ -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 ;-/

Ouch.

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.

Yes, I was unable to find any others either.

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):

Thanks, will run some checks for good measure.
But the fix below looks good to me.