Re: [PATCH] fs: correctly check for errors from replace_fd() in receive_fd_replace()

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Aug 01, 2025 at 12:48:09PM +0200, Jan Kara wrote:
> On Fri 01-08-25 09:38:38, Thomas Weißschuh wrote:
> > replace_fd() returns either a negative error number or the number of the
> > new file descriptor. The current code misinterprets any positive file
> > descriptor number as an error.
> > 
> > Only check for negative error numbers, so that __receive_sock() is called
> > correctly for valid file descriptors.
> > 
> > Fixes: 173817151b15 ("fs: Expand __receive_fd() to accept existing fd")
> > Fixes: 42eb0d54c08a ("fs: split receive_fd_replace from __receive_fd")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> 
> Indeed. I'm wondering how come nobody noticed...

One word: seccomp.  Considering the background amount of bogus userland
behaviour coming with it, I wouldn't expect a... vigorous test coverage
for that one ;-/

It's definitely a bug that needs fixing, but I'm not sure this is the right
way to fix it.

Look: replace_fd(fd, file, flags) returns fd on success and -E... on failure.
Not a single user cares which non-negative value had been returned.  What's
more, "returns fd on success" is a side effect of using do_dup2() and
being lazy about it.

And the entire thing is not on any hot paths.  So I suspect that a better fix
would be

	err = do_dup2(files, file, fd, flags);
	if (err < 0)
		return err;
	return 0;

in replace_fd() in place of
	return do_dup2(files, file, fd, flags);

so we don't invite more surprises like that.