Re: [RFC 1/1] shiftfs: uid/gid shifting bind mount

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Tue, 2016-05-17 at 06:23 -0400, James Bottomley wrote:
> On Mon, 2016-05-16 at 22:47 -0500, Serge E. Hallyn wrote:
> > On Mon, May 16, 2016 at 10:28:32PM -0400, James Bottomley wrote:
> > > On Mon, 2016-05-16 at 19:41 +0000, Serge Hallyn wrote:
> > > > Hey James,
> > > > 
> > > > I probably did something wrong - but i applied your patch onto
> > > > 4.6,
> > > > compiled in shiftfs, did
> > > > 
> > > > mount -t shiftfs -o uidmap=0:100000:65536,gidmap=0:100000:65536
> > > > /home/ubuntu /mnt
> > > > 
> > > > and ls segfaults and gives me kernel syslog msgs like:
> > > 
> > > Hm, it looks to be something IMA related, since the SUSE default
> > > is
> > > no
> > > IMA and this BUG in the filesystem is to do with the IMA version
> > > of
> > > i_readcount_dec.  I'll recompile my kernel to see if I can
> > > reproduce. 
> > >  Just in case, what's the underlying filesystem on /home/ubuntu?
> > 
> > It was ext4
> 
> Thanks.  I've got it to reproduce with CONFIG_IMA set ... just
> debugging now.

OK, I think this is the fix, can you apply on top of what you have
(it's two fixes, one for the RCU lookup and the other for the IMA
problem).

This probably has to be fixed in the VFS, but at least it will prove
I've got the correct problem and diagnosis.

Thanks,

James

---

diff --git a/fs/shiftfs.c b/fs/shiftfs.c
index d352377..2699b95 100644
--- a/fs/shiftfs.c
+++ b/fs/shiftfs.c
@@ -525,6 +525,9 @@ static int shiftfs_permission(struct inode *inode, int mask)
 	int err;
 	const struct cred *oldcred, *newcred;
 
+	if (mask & MAY_NOT_BLOCK)
+		return -ECHILD;
+
 	oldcred = shiftfs_new_creds(&newcred, inode->i_sb);
 	if (iop->permission)
 		err = iop->permission(reali, mask);
@@ -598,6 +601,15 @@ static int shiftfs_release(struct inode *inode, struct file *file)
 	if (sfc->release)
 		err = sfc->release(inode, file);
 
+#ifdef CONFIG_IMA
+	/* FIXME: IMA calls aren't balanced across ->open ->release
+	 * they occur after ->open and after ->release, so manually
+	 * swizzle here */
+
+	if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
+		i_readcount_dec(sfc->inode);
+#endif
+
 	file->f_inode = sfc->inode;
 	file->f_op = sfc->inode->i_fop;
 	fops_put(inode->i_fop);
@@ -631,6 +643,16 @@ static int shiftfs_open(struct inode *inode, struct file *file)
 	file->f_op = &sfc->fop;
 	file->f_inode = reali;
 
+#ifdef CONFIG_IMA
+	/* FIXME: IMA calls always operate on a saved copy of the
+	 * inode so they increment the above and decrement the
+	 * underlying. fix that here */
+
+	if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
+		i_readcount_inc(reali);
+#endif
+
+
 	if (fop->open)
 		err = fop->open(reali, file);