Thread (12 messages) 12 messages, 3 authors, 2016-01-22

Re: [PATCH 2/7] quota: add new quotactl Q_XGETNEXTQUOTA

From: Eric Sandeen <hidden>
Date: 2016-01-22 13:57:21
Also in: linux-fsdevel

On 1/22/16 2:55 AM, Jan Kara wrote:
On Thu 21-01-16 22:07:19, Eric Sandeen wrote:
quoted
Q_XGETNEXTQUOTA is exactly like Q_XGETQUOTA, except that it
will return quota information for the id equal to or greater
than the id requested.  In other words, if the requested id has
no quota, the command will return quota information for the
next higher id which does have a quota set.  If no higher id
has an active quota, -ESRCH is returned.

This allows filesystems to do efficient iteration in kernelspace,
much like extN filesystems do in userspace when asked to report
all active quotas.

The patch adds a d_id field to struct qc_dqblk so that we can
pass back the id of the quota which was found, and return it
to userspace.

Today, filesystems such as XFS require getpwent-style iterations,
and for systems which have i.e. LDAP backends, this can be very
slow, or even impossible if iteration is not allowed in the
configuration.

Signed-off-by: Eric Sandeen <redacted>
...
quoted
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index ea66670..4bf8d40 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -33,6 +33,7 @@ static int check_quotactl_permission(struct super_block *sb, int type, int cmd,
 	/* allow to query information for dquots we "own" */
 	case Q_GETQUOTA:
 	case Q_XGETQUOTA:
+	case Q_XGETNEXTQUOTA:
IMO you should require CAP_SYS_ADMIN for the quotactl. Definitely doing the
UID and GID checks for GETNEXTQUOTA looks strange to me since the returned
structure may be for a different ID. Or did you assume that existing user
will have quota structure allocated so we always return quotas for that ID
in that case? I'm not sure this is good to rely on...
Oh whoops.  OK that was dumb of me, thanks for catching it.  No, I
didn't intend to rely on the asking user having a quota, it was
just a dumb mistake.  :)
quoted
+	ret = sb->s_qcop->get_nextdqblk(sb, qid, &qdq);
+	if (ret)
+	        return ret;
+	copy_to_xfs_dqblk(&fdq, &qdq, type, qdq.d_id);
+	if (copy_to_user(addr, &fdq, sizeof(fdq)))
+	        return -EFAULT;
+	return ret;
+}
So how about passing pointer to 'qid' to ->get_nextdqblk() and return the ID
that way? That will also force you to fix the issue that you currently
completely miss user-namespace conversions for the ID ;).
Ok.
I definitely dislike mixing d_id in the qdq structure with arguments of fs
callbacks and that d_id doesn't get filled for most callbacks. That is going
to cause confusion.
Yeah, fair enough.  I'll change it.

Thanks,
-Eric
								Honza
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help