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@xxxxxxxxxxxxxxx
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
>
> 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.
