On 1/22/16 2:55 AM, Jan Kara wrote:
> On Thu 21-01-16 22:07:19, Eric Sandeen wrote:
>> 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 <sandeen@xxxxxxxxxx>
> ...
>> 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. :)
>> + 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
>
|