Re: [PATCH 1/8] quota: Allow to pass mount path to quotactl

[Jan Kara <jack@suse.cz>]

On Fri 24-01-20 14:13:16, Sascha Hauer wrote:
> This patch introduces the Q_PATH flag to the quotactl cmd argument.
> When given, the path given in the special argument to quotactl will
> be the mount path where the filesystem is mounted, instead of a path
> to the block device.
> This is necessary for filesystems which do not have a block device as
> backing store. Particularly this is done for upcoming UBIFS support.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Thanks for the patch. Some comments are below.

> @@ -821,15 +822,20 @@ int kernel_quotactl(unsigned int cmd, const char __user *special,
>  {
>  	uint cmds, type;
>  	struct super_block *sb = NULL;
> -	struct path path, *pathp = NULL;
> +	struct path file_path, *file_pathp = NULL, sb_path;
>  	int ret;
> +	bool q_path;
>  
>  	cmds = cmd >> SUBCMDSHIFT;
>  	type = cmd & SUBCMDMASK;
>  
> +

Unnecessary empty line added...

>  	if (type >= MAXQUOTAS)
>  		return -EINVAL;
>  
> +	q_path = cmds & Q_PATH;
> +	cmds &= ~Q_PATH;
> +
>  	/*
>  	 * As a special case Q_SYNC can be called without a specific device.
>  	 * It will iterate all superblocks that have quota enabled and call
> @@ -847,28 +853,45 @@ int kernel_quotactl(unsigned int cmd, const char __user *special,
>  	 * resolution (think about autofs) and thus deadlocks could arise.
>  	 */
>  	if (cmds == Q_QUOTAON) {
> -		ret = user_path_at(AT_FDCWD, addr, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &path);
> +		ret = user_path_at(AT_FDCWD, addr,
> +				   LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT,
> +				   &file_path);
>  		if (ret)
> -			pathp = ERR_PTR(ret);
> +			file_pathp = ERR_PTR(ret);
>  		else
> -			pathp = &path;
> +			file_pathp = &file_path;
>  	}
>  
> -	sb = quotactl_block(special, cmds);
> -	if (IS_ERR(sb)) {
> -		ret = PTR_ERR(sb);
> -		goto out;
> +	if (q_path) {
> +		ret = user_path_at(AT_FDCWD, special,
> +				   LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT,
> +				   &sb_path);
> +		if (ret)
> +			goto out;
> +
> +		sb = sb_path.mnt->mnt_sb;

So I've realized that just looking up superblock with user_path_at() is not
enough. Quota code also expects that the superblock will be locked
(sb->s_umount) and filesystem will not be frozen (in case the quota
operation is going to modify the filesystem). This is needed to serialize
e.g. remount and quota operations or quota operations among themselves.
So you still need something like following to get superblock from the path:

static int quotactl_path(const char __user *special, int cmd, struct path *path)
{
	struct super_block *sb;

	ret = user_path_at(AT_FDCWD, special, LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT,
			   path);
	if (ret)
		return ret;
	sb = sb_path.mnt->mnt_sb;
restart:
	if (quotactl_cmd_onoff(cmd))
		down_write(&sb->s_umount);
	else
		down_read(&sb->s_umount);

	if (quotactl_cmd_write(cmd) && sb->s_writers.frozen != SB_UNFROZEN) {
		if (quotactl_cmd_onoff(cmd))
			up_write(&sb->s_umount);
		else
			up_read(&sb->s_umount);
		wait_event(sb->s_writers.wait_unfrozen,
			   sb->s_writers.frozen == SB_UNFROZEN);
		goto restart;

	}
	return sb;
}

And then appropriate counterparts when releasing the superblock.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR