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

On Tue, Jul 05, 2016 at 04:28:53PM -0500, Eric W. Biederman wrote:

Dave Chinner [off-list ref] writes:

quoted

On Tue, Jul 05, 2016 at 10:34:49AM -0500, Eric W. Biederman wrote:

quoted

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.

Which is the same situation currently for every filesystem that
supports quotas, regardless of the implementation infrastructure.

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.

All the more reason you should be adding the same guard to all the
other filesystems....

All i'm asking you to do is to make this check in a way that all
filesystems that implement quotas will execute it. Don't leave
landmines with security implications around - make sure all
filesystems have the same protections.

-Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org