| Message ID | 1406067727-19683-77-git-send-email-kamal@canonical.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 22, 2014 at 03:21:27PM -0700, Kamal Mostafa wrote: > 3.8.13.27 -stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Dwight Engen <dwight.engen@oracle.com> > > commit fd5e2aa8653665ae1cc60f7aca1069abdbcad3f6 upstream. > > Use inode_capable() to check if SUID|SGID bits should be cleared to match > similar check in inode_change_ok(). > > The check for CAP_LINUX_IMMUTABLE was not modified since all other file > systems also check against init_user_ns rather than current_user_ns. > > Only allow changing of projid from init_user_ns. > > Reviewed-by: Dave Chinner <dchinner@redhat.com> > Reviewed-by: Gao feng <gaofeng@cn.fujitsu.com> > Signed-off-by: Dwight Engen <dwight.engen@oracle.com> > Signed-off-by: Ben Myers <bpm@sgi.com> > [ kamal: 3.8-stable prereq for > 23adbe1 fs,userns: Change inode_capable to capable_wrt_inode_uidgid ] > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > --- > fs/xfs/xfs_ioctl.c | 11 +++++++++-- > kernel/capability.c | 1 + > 2 files changed, 10 insertions(+), 2 deletions(-) Why are you backporting this to 3.8? namespace support didn't come along until much later, so grabbing one patch out of themiddle of a patch series to allow userns support in XFS is likely to cause problems because there's no supporting code in XFS it. Please don't randomly cherry pick userns support patches that change permission checks back into kernels that don't have userns support. Cheers, Dave.
On Wed, 2014-07-23 at 09:12 +1000, Dave Chinner wrote: > On Tue, Jul 22, 2014 at 03:21:27PM -0700, Kamal Mostafa wrote: > > 3.8.13.27 -stable review patch. If anyone has any objections, please let me know. > > > > ------------------ > > > > From: Dwight Engen <dwight.engen@oracle.com> > > > > commit fd5e2aa8653665ae1cc60f7aca1069abdbcad3f6 upstream. > > > > Use inode_capable() to check if SUID|SGID bits should be cleared to match > > [...] > > Why are you backporting this to 3.8? namespace support didn't come > along until much later, so grabbing one patch out of themiddle of a > patch series to allow userns support in XFS is likely to cause > problems because there's no supporting code in XFS it. > > Please don't randomly cherry pick userns support patches that change > permission checks back into kernels that don't have userns support. Yup, that's why we ask for reviews all right! I've dropped these from the 3.8-stable queue: fs,userns: Change inode_capable to capable_wrt_inode_uidgid xfs: ioctl check for capabilities in the current user namespace Thanks Dave, -Kamal
Kamal Mostafa <kamal@canonical.com> writes: > On Wed, 2014-07-23 at 09:12 +1000, Dave Chinner wrote: >> On Tue, Jul 22, 2014 at 03:21:27PM -0700, Kamal Mostafa wrote: >> > 3.8.13.27 -stable review patch. If anyone has any objections, please let me know. >> > >> > ------------------ >> > >> > From: Dwight Engen <dwight.engen@oracle.com> >> > >> > commit fd5e2aa8653665ae1cc60f7aca1069abdbcad3f6 upstream. >> > >> > Use inode_capable() to check if SUID|SGID bits should be cleared to match >> > [...] >> >> Why are you backporting this to 3.8? namespace support didn't come >> along until much later, so grabbing one patch out of themiddle of a >> patch series to allow userns support in XFS is likely to cause >> problems because there's no supporting code in XFS it. >> >> Please don't randomly cherry pick userns support patches that change >> permission checks back into kernels that don't have userns support. > > Yup, that's why we ask for reviews all right! I've dropped these from > the 3.8-stable queue: > > fs,userns: Change inode_capable to capable_wrt_inode_uidgid The fs,userns: Chage inode_capable to capable_wrt_inode_uidgid is appropriate for 3.8. I think that one is applicable all of the way back to 3.4 I don't know if xfs in 3.8 called inode_capable in xfs and if it didn't you can remove that hunk. To keep things very simple you could just skip the rename of inode_capable to capable_wrt_uidgid and just include the one line change to add kgid_has_mapping. But that bug fix is very much applicable to older kernels. Eric
On Wed, 2014-07-23 at 18:51 -0700, Eric W. Biederman wrote: > The fs,userns: Chage inode_capable to capable_wrt_inode_uidgid is > appropriate for 3.8. I think that one is applicable all of the way > back to 3.4 > > I don't know if xfs in 3.8 called inode_capable in xfs and if it didn't > you can remove that hunk. Okay, yup, that's what was done in the 3.10-stable port which I should have found and used in the first place. So I have now queued that (fs,userns: Change inode_capable to capable_wrt_inode_uidgid) up again for 3.8-stable. Thanks very much, Eric. I appreciate you not letting this one slip past me! -Kamal > To keep things very simple you could just > skip the rename of inode_capable to capable_wrt_uidgid and just > include the one line change to add kgid_has_mapping. > > But that bug fix is very much applicable to older kernels. > > Eric >
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index cdaef2d..ec74a78 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -976,15 +976,22 @@ xfs_ioctl_setattr( * to the file owner ID, except in cases where the * CAP_FSETID capability is applicable. */ - if (current_fsuid() != ip->i_d.di_uid && !capable(CAP_FOWNER)) { + if (!inode_owner_or_capable(VFS_I(ip))) { code = XFS_ERROR(EPERM); goto error_return; } /* * Do a quota reservation only if projid is actually going to change. + * Only allow changing of projid from init_user_ns since it is a + * non user namespace aware identifier. */ if (mask & FSX_PROJID) { + if (current_user_ns() != &init_user_ns) { + code = XFS_ERROR(EINVAL); + goto error_return; + } + if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp) && xfs_get_projid(ip) != fa->fsx_projid) { @@ -1098,7 +1105,7 @@ xfs_ioctl_setattr( * cleared upon successful return from chown() */ if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) && - !capable(CAP_FSETID)) + !inode_capable(VFS_I(ip), CAP_FSETID)) ip->i_d.di_mode &= ~(S_ISUID|S_ISGID); /* diff --git a/kernel/capability.c b/kernel/capability.c index f6c2ce5..a4b6744 100644 --- a/kernel/capability.c +++ b/kernel/capability.c @@ -464,3 +464,4 @@ bool inode_capable(const struct inode *inode, int cap) return ns_capable(ns, cap) && kuid_has_mapping(ns, inode->i_uid); } +EXPORT_SYMBOL(inode_capable);