Re: [PATCH] loop: reintroduce global lock for safe loop_validate_file() traversal

[Christoph Hellwig <hch@lst.de>]

> +static int loop_global_lock_killable(struct loop_device *lo, bool global)
> +{
> +	int err;
> +
> +	if (global) {
> +		err = mutex_lock_killable(&loop_validate_mutex);
> +		if (err)
> +			return err;
> +	}
> +	err = mutex_lock_killable(&lo->lo_mutex);
> +	if (err && global)
> +		mutex_unlock(&loop_validate_mutex);
> +	return err;
> +}
> +
> +static void loop_global_unlock(struct loop_device *lo, bool global)
> +{
> +	mutex_unlock(&lo->lo_mutex);
> +	if (global)
> +		mutex_unlock(&loop_validate_mutex);
> +}

Comments explaining these functions would be nice.

>  static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
>  			  unsigned int arg)
>  {
> -	struct file	*file = NULL, *old_file;
> +	struct file *file = fget(arg);
> +	struct file *old_file;
>  	int		error;
>  	bool		partscan;
> +	bool is_loop;

This doesn't match the existing formatting.  Either keep the existing
one or change the few other variables as well.

> +	is_loop = is_loop_device(file);
> +	error = loop_global_lock_killable(lo, is_loop);
> +	if (error) {
> +		fput(file);
>  		return error;
> +	}

Please use a goto label to share the fput with the other error handling.

> @@ -1143,6 +1167,7 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,
>  	loff_t		size;
>  	bool		partscan;
>  	unsigned short  bsize;
> +	bool is_loop;

Please follow the existing formatting.

Otherwise this looks reasonable to me.