Re: [PATCH 2/3] coredump: fix error handling for replace_fd()

[Oleg Nesterov]

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.

Oleg.