On Mon, Apr 14, 2025 at 02:11:56PM +0200, Oleg Nesterov wrote:
> On 04/14, Christian Brauner wrote:
> >
> > The replace_fd() helper returns the file descriptor number on success
> > and a negative error code on failure. The current error handling in
> > umh_pipe_setup() only works because the file descriptor that is replaced
> > is zero but that's pretty volatile. Explicitly check for a negative
> > error code.
>
> ...
>
> > @@ -515,6 +517,9 @@ static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
> >
> > err = replace_fd(0, files[0], 0);
> > fput(files[0]);
> > + if (err < 0)
> > + return err;
> > +
> > /* and disallow core files too */
> > current->signal->rlim[RLIMIT_CORE] = (struct rlimit){1, 1};
>
> The patch looks trivial and correct, but if we do not want to rely on
> the fact that replace_fd(fd => 0) return 0 on sucess, then this patch
> should also do
>
> - return err;
> + return 0;
>
> ?
>
> otherwise this cleanup looks "incomplete" to me.
Ok, done.
