Re: [PATCH] fcntl: make F_DUPFD_QUERY associative

[Lennart Poettering <lennart@poettering.net>]

On Di, 08.10.24 13:30, Christian Brauner (brauner@kernel.org) wrote:

> Currently when passing a closed file descriptor to
> fcntl(fd, F_DUPFD_QUERY, fd_dup) the order matters:
>
>     fd = open("/dev/null");
>     fd_dup = dup(fd);
>
> When we now close one of the file descriptors we get:
>
>     (1) fcntl(fd, fd_dup) // -EBADF
>     (2) fcntl(fd_dup, fd) // 0 aka not equal
>
> depending on which file descriptor is passed first. That's not a huge
> deal but it gives the api I slightly weird feel. Make it so that the
> order doesn't matter by requiring that both file descriptors are valid:
>
> (1') fcntl(fd, fd_dup) // -EBADF
> (2') fcntl(fd_dup, fd) // -EBADF
>
> Fixes: c62b758bae6a ("fcntl: add F_DUPFD_QUERY fcntl()")
> Cc: <stable@vger.kernel.org>
> Reported-by: Lennart Poettering <lennart@poettering.net>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/fcntl.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index 22dd9dcce7ec..3d89de31066a 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -397,6 +397,9 @@ static long f_dupfd_query(int fd, struct file *filp)
>  {
>  	CLASS(fd_raw, f)(fd);
>
> +	if (fd_empty(f))
> +		return -EBADF;
> +
>  	/*
>  	 * We can do the 'fdput()' immediately, as the only thing that
>  	 * matters is the pointer value which isn't changed by the fdput.

Thanks! LGTM!

Reviewed-By: Lennart Poettering <lennart@poettering.net>

Lennart

--
Lennart Poettering, Berlin