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

[Eric W. Biederman]

Dave Chinner <david at fromorbit.com> writes:

> On Tue, Jul 05, 2016 at 10:34:49AM -0500, Eric W. Biederman wrote:
>> The more I think of it the more I think that sounds like wisdom.
>> Dropping this patch and replacing it by one that just does:
>> 
>> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
>> index d8fb0fd3ff6f..9c9890fe18b7 100644
>> --- a/fs/quota/dquot.c
>> +++ b/fs/quota/dquot.c
>> @@ -2273,6 +2273,11 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
>>                 error = -EINVAL;
>>                 goto out_fmt;
>>         }
>> +       /* Filesystems outside of init_user_ns not yet supported */
>> +       if (sb->s_user_ns != &init_user_ns) {
>> +               error = -EINVAL;
>> +               goto out_fmt;
>> +       }
>>         /* Usage always has to be set... */
>>         if (!(flags & DQUOT_USAGE_ENABLED)) {
>>                 error = -EINVAL;
>> 
>> 
>> seems a lot more appropriate at this point.  That is enough to give a
>> great big hint there is something that needs to be done but won't
>> embrittle the code with untested corner cases.
>
> You'll need to propagate that to all filesystems that have their own
> quota implemenation, too.

All of the filesytems that have their own quota implementations omit the
flag FS_USERNS_MOUNT in fs_flags in struct filesystem so they are
protected.

The above change is extending to the generic implementation of quota
files the same protection that is already enjoyed by filesystems in
general.  Recognition that being attacked by malicious users is a
difficult thing to defend against, and not enabling the code until there
is a reasonable certainty the code is robust against maliciuos attack
and everyone involved with maintaining the code is comfortable about the
situtation.

I have every intention of keeping all of the changes to the generic
parts of the quota layer so that filesystems who wish to implement
unprivileged mounts and implement quotas may proceed if they so desire.

Eric

p.s.  I  expect the the generic quota implementation is simple enough
that it is not particularly suseptible to problems caused by malicious
data.  But I don't currently care enough to verify and test that
assumption so this is very much the wrong time for me to be enabling the
feature.