Re: [PATCH v2 review 09/11] quota: Handle quota data stored in s_user_ns.

[Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>]

On Sat 02-07-16 12:33:29, Eric W. Biederman wrote:
> In Q_XSETQLIMIT use sb->s_user_ns to detect when we are dealing with
> the filesystems notion of id 0.

Hum, is it really usable? Basically the tool calling Q_XSETQLIMIT would
have to be aware of the namespace the filesystem is mounted in to be able
to perform the desired operation (and if it gets is wrong, there's
possibility it would just silently set the timers for some user instead of
for all users).

> For the dquot hashfn translate the qid into sb->s_user_ns to remove
> extraneous bits before hashing.
> 
> When dealing with generic quota files in quota_tree.c, quota_v1.c, and
> quota_v2.c before writing quotas to the file encode them in
> sb->s_user_ns, and decode ids read from the file from sb->s_user_ns.

And here the sb->s_user_ns becomes part of the on-disk format because the
numerical ID value is used to compute the position in on-disk data
structures. This seems to be in conflict with the idea that anything that
is stored on disk is stored in the initial user namespace.

> One complication comes up in qtree_get_next_id, as it is possible in
> principle for id's to be present in the quota file that do not have a
> mapping in the filesystems user namespace.  If that is the case the
> code is modified to skip the unmapped ids.

So I'm not 100% sure how this is going to work. qtree_get_next_id() is used
as an interface to report quota information for the whole filesystem. So
skipping ids without mappings makes sense. However userspace uses the
interface as:

id = 0;
while (get_next_quota(id, &result)) {
	do stuff;
	id = result.dq_id + 1;
}

If sb->s_user_ns is the current namespace of the process, I suppose this is
going to work as expected. If the namespace of the process is different
(including the init_user_ns), I expect this breaks, doesn't it?

								Honza
-- 
Jan Kara <jack-IBi9RG/b67k@public.gmane.org>
SUSE Labs, CR