Re: [PATCH v3 10/11] make __set_open_fd() set cloexec state as well

[Steven Price <steven.price@arm.com>]

On 07/10/2024 18:43, Al Viro wrote:
> ->close_on_exec[] state is maintained only for opened descriptors;
> as the result, anything that marks a descriptor opened has to
> set its cloexec state explicitly.
> 
> As the result, all calls of __set_open_fd() are followed by
> __set_close_on_exec(); might as well fold it into __set_open_fd()
> so that cloexec state is defined as soon as the descriptor is
> marked opened.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/file.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/file.c b/fs/file.c
> index d8fccd4796a9..b63294ed85ec 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -248,12 +248,13 @@ static inline void __set_close_on_exec(unsigned int fd, struct fdtable *fdt,
>  	}
>  }
>  
> -static inline void __set_open_fd(unsigned int fd, struct fdtable *fdt)
> +static inline void __set_open_fd(unsigned int fd, struct fdtable *fdt, bool set)
>  {
>  	__set_bit(fd, fdt->open_fds);
>  	fd /= BITS_PER_LONG;

Here fd is being modified...

>  	if (!~fdt->open_fds[fd])
>  		__set_bit(fd, fdt->full_fds_bits);
> +	__set_close_on_exec(fd, fdt, set);

... which means fd here isn't the same as the passed in value. So this
call to __set_close_on_exec affects a different fd to the expected one.

Steve

>  }
>  
>  static inline void __clear_open_fd(unsigned int fd, struct fdtable *fdt)
> @@ -517,8 +518,7 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags)
>  	if (start <= files->next_fd)
>  		files->next_fd = fd + 1;
>  
> -	__set_open_fd(fd, fdt);
> -	__set_close_on_exec(fd, fdt, flags & O_CLOEXEC);
> +	__set_open_fd(fd, fdt, flags & O_CLOEXEC);
>  	error = fd;
>  
>  out:
> @@ -1186,8 +1186,7 @@ __releases(&files->file_lock)
>  		goto Ebusy;
>  	get_file(file);
>  	rcu_assign_pointer(fdt->fd[fd], file);
> -	__set_open_fd(fd, fdt);
> -	__set_close_on_exec(fd, fdt, flags & O_CLOEXEC);
> +	__set_open_fd(fd, fdt, flags & O_CLOEXEC);
>  	spin_unlock(&files->file_lock);
>  
>  	if (tofree)