Commit 2282a9e6 authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Darrick J. Wong
Browse files

xfs: use strndup_user in XFS_IOC_ATTRMULTI_BY_HANDLE 



Simplify the user copy code by using strndup_user.  This means that we
now do one memory allocation per operation instead of one per ioctl,
but memory allocations are cheap compared to the actual file system
operations.  Also the error for an invalid path is now EINVAL or EFAULT
instead of the previous odd and undocumented ERANGE.

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarChandan Rajendra <chandanrlinux@gmail.com>
Reviewed-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
parent 6cc4f4ff

fs/xfs/xfs_ioctl.c

+5 −12
Original line number Diff line number Diff line
@@ -448,11 +448,6 @@ xfs_attrmulti_by_handle( 
		goto out_dput;

	}



	error = -ENOMEM;

	attr_name = kmalloc(MAXNAMELEN, GFP_KERNEL);

	if (!attr_name)

		goto out_kfree_ops;



	error = 0;

	for (i = 0; i < am_hreq.opcount; i++) {

		if ((ops[i].am_flags & ATTR_ROOT) &&
@@ -462,12 +457,11 @@ xfs_attrmulti_by_handle( 
		}

		ops[i].am_flags &= ~ATTR_KERNEL_FLAGS;



		ops[i].am_error = strncpy_from_user((char *)attr_name,

				ops[i].am_attrname, MAXNAMELEN);

		if (ops[i].am_error == 0 || ops[i].am_error == MAXNAMELEN)

			error = -ERANGE;

		if (ops[i].am_error < 0)

		attr_name = strndup_user(ops[i].am_attrname, MAXNAMELEN);

		if (IS_ERR(attr_name)) {

			ops[i].am_error = PTR_ERR(attr_name);

			break;

		}



		switch (ops[i].am_opcode) {

		case ATTR_OP_GET:
@@ -498,13 +492,12 @@ xfs_attrmulti_by_handle( 
		default:

			ops[i].am_error = -EINVAL;

		}

		kfree(attr_name);

	}



	if (copy_to_user(am_hreq.ops, ops, size))

		error = -EFAULT;



	kfree(attr_name);

 out_kfree_ops:

	kfree(ops);

 out_dput:

	dput(dentry);

fs/xfs/xfs_ioctl32.c

+5 −12
Original line number Diff line number Diff line
@@ -445,11 +445,6 @@ xfs_compat_attrmulti_by_handle( 
		goto out_dput;

	}



	error = -ENOMEM;

	attr_name = kmalloc(MAXNAMELEN, GFP_KERNEL);

	if (!attr_name)

		goto out_kfree_ops;



	error = 0;

	for (i = 0; i < am_hreq.opcount; i++) {

		if ((ops[i].am_flags & ATTR_ROOT) &&
@@ -459,13 +454,12 @@ xfs_compat_attrmulti_by_handle( 
		}

		ops[i].am_flags &= ~ATTR_KERNEL_FLAGS;



		ops[i].am_error = strncpy_from_user((char *)attr_name,

				compat_ptr(ops[i].am_attrname),

		attr_name = strndup_user(compat_ptr(ops[i].am_attrname),

				MAXNAMELEN);

		if (ops[i].am_error == 0 || ops[i].am_error == MAXNAMELEN)

			error = -ERANGE;

		if (ops[i].am_error < 0)

		if (IS_ERR(attr_name)) {

			ops[i].am_error = PTR_ERR(attr_name);

			break;

		}



		switch (ops[i].am_opcode) {

		case ATTR_OP_GET:
@@ -496,13 +490,12 @@ xfs_compat_attrmulti_by_handle( 
		default:

			ops[i].am_error = -EINVAL;

		}

		kfree(attr_name);

	}



	if (copy_to_user(compat_ptr(am_hreq.ops), ops, size))

		error = -EFAULT;



	kfree(attr_name);

 out_kfree_ops:

	kfree(ops);

 out_dput:

	dput(dentry);