diff mbox series

[SRU,Disco] shiftfs: simplify passthrough verification

Message ID 20190414205258.25263-1-christian.brauner@ubuntu.com
State New
Headers show
Series [SRU,Disco] shiftfs: simplify passthrough verification | expand

Commit Message

Christian Brauner April 14, 2019, 8:52 p.m. UTC
Copy up the passthrough mount settings of the mark mount point to the
shiftfs overlay.

Before this commit we used to keep a reference to the shiftfs mark
mount's shiftfs_super_info which was stashed in the superblock of the
mark mount. The problem is that we only take a reference to the mount of
the underlay, i.e. the filesystem that is *under* the shiftfs mark
mount. This means when someone performs a shiftfs mark mount, then a
shiftfs overlay mount and then immediately unmounts the shiftfs mark
mount we muck with invalid memory since shiftfs_put_super might have
already been called freeing that memory.
One solution would be to start reference counting which is overkill. We
only care about the passthrough mount option of the mark mount. And we
only need it to verify that on remount the new passthrough options of
the shiftfs overlay are a subset of the mark mount's passthrough
options. In other scenarios we don't care. So copying up is good enough
and also only needs to happen once on mount, i.e. when a new superblock
is created and the .fill_super method is called.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/shiftfs.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

Comments

Thadeu Lima de Souza Cascardo April 15, 2019, 11:34 a.m. UTC | #1
Hi, Christian.

TL;DR: why should we apply this fix at this point of the release?

Is this bug upstream at all? If not, it should have a subject of "UBUNTU:
SAUCE".

The main issue with this submission is that it does not have a buglink with a
bug that follows the SRU process, that is, has a good description of the
impact, regression potential and what test has been done for both the fix and
those potential regressions.

Thanks.
Cascardo.
Christian Brauner April 15, 2019, 11:48 a.m. UTC | #2
Hi Cascardo,

On Mon, Apr 15, 2019 at 08:34:44AM -0300, Thadeu Lima de Souza Cascardo wrote:
> Hi, Christian.
> 
> TL;DR: why should we apply this fix at this point of the release?

Ah, sorry. I just wanted to have a fix to review out. Sorry for not
following proper protocol.

> 
> Is this bug upstream at all? If not, it should have a subject of "UBUNTU:
> SAUCE".
> 
> The main issue with this submission is that it does not have a buglink with a
> bug that follows the SRU process, that is, has a good description of the
> impact, regression potential and what test has been done for both the fix and
> those potential regressions.

Ok, I'll rework this now and make this suitable for an SRU.

Thanks!
Christian
Christian Brauner April 15, 2019, 12:36 p.m. UTC | #3
On Mon, Apr 15, 2019 at 01:48:09PM +0200, Christian Brauner wrote:
> Hi Cascardo,
> 
> On Mon, Apr 15, 2019 at 08:34:44AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > Hi, Christian.
> > 
> > TL;DR: why should we apply this fix at this point of the release?

Just to clarify, I did not mean to imply that a new point release is
supposed to be made. Rather just that this is an SRU. There seems to
have been some confusion there.

The rest of your points obviously still apply.

Thanks!
Christian
diff mbox series

Patch

diff --git a/fs/shiftfs.c b/fs/shiftfs.c
index 8e064756ea0c..4c8a6ec2a617 100644
--- a/fs/shiftfs.c
+++ b/fs/shiftfs.c
@@ -28,7 +28,7 @@  struct shiftfs_super_info {
 	const struct cred *creator_cred;
 	bool mark;
 	unsigned int passthrough;
-	struct shiftfs_super_info *info_mark;
+	unsigned int passthrough_mark;
 };
 
 struct shiftfs_file_info {
@@ -52,10 +52,6 @@  static inline bool shiftfs_passthrough_ioctls(struct shiftfs_super_info *info)
 	if (!(info->passthrough & SHIFTFS_PASSTHROUGH_IOCTL))
 		return false;
 
-	if (info->info_mark &&
-	    !(info->info_mark->passthrough & SHIFTFS_PASSTHROUGH_IOCTL))
-		return false;
-
 	return true;
 }
 
@@ -64,10 +60,6 @@  static inline bool shiftfs_passthrough_statfs(struct shiftfs_super_info *info)
 	if (!(info->passthrough & SHIFTFS_PASSTHROUGH_STAT))
 		return false;
 
-	if (info->info_mark &&
-	    !(info->info_mark->passthrough & SHIFTFS_PASSTHROUGH_STAT))
-		return false;
-
 	return true;
 }
 
@@ -1824,7 +1816,7 @@  static int shiftfs_remount(struct super_block *sb, int *flags, char *data)
 
 	if (info->passthrough != new.passthrough) {
 		/* Don't allow exceeding passthrough options of mark mount. */
-		if (!passthrough_is_subset(info->info_mark->passthrough,
+		if (!passthrough_is_subset(info->passthrough_mark,
 					   info->passthrough))
 			return -EPERM;
 
@@ -1926,9 +1918,19 @@  static int shiftfs_fill_super(struct super_block *sb, void *raw_data,
 
 			sbinfo->mnt = mntget(sbinfo_mp->mnt);
 			dentry = dget(path.dentry->d_fsdata);
+			/*
+			 * Copy up the passthrough mount options from the
+			 * parent mark mountpoint.
+			 */
+			sbinfo->passthrough_mark = sbinfo_mp->passthrough_mark;
 		} else {
 			sbinfo->mnt = mntget(path.mnt);
 			dentry = dget(path.dentry);
+			/*
+			 * For a new mark passthrough_mark and passthrough
+			 * are identical.
+			 */
+			sbinfo->passthrough_mark = sbinfo->passthrough;
 		}
 
 		sbinfo->creator_cred = prepare_creds();
@@ -1956,7 +1958,12 @@  static int shiftfs_fill_super(struct super_block *sb, void *raw_data,
 		sbinfo->mnt = mntget(sbinfo_mp->mnt);
 		sbinfo->creator_cred = get_cred(sbinfo_mp->creator_cred);
 		dentry = dget(path.dentry->d_fsdata);
-		sbinfo->info_mark = sbinfo_mp;
+		/*
+		 * Copy up passthrough settings from mark mountpoint so we can
+		 * verify when the overlay wants to remount with different
+		 * passthrough settings.
+		 */
+		sbinfo->passthrough_mark = sbinfo_mp->passthrough;
 	}
 
 	sb->s_stack_depth = dentry->d_sb->s_stack_depth + 1;