linux-kernel - Re: [PATCH] Add MS_BIND

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 14 Feb 2008 01:02:01 -0500
From:	Christoph Hellwig <hch@...radead.org>
To:	Paul Menage <menage@...gle.com>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Add MS_BIND_FLAGS mount flag

On Tue, Feb 12, 2008 at 09:45:15PM -0800, Paul Menage wrote:
> From: Paul Menage <menage@...gle.com>
>
> Add a new mount() flag, MS_BIND_FLAGS.
>
> MS_BIND_FLAGS indicates that a bind mount should take its per-mount flags
> from the arguments passed to mount() rather than from the source
> mountpoint.
>
> This flag allows you to create a bind mount with the desired per-mount
> flags in a single operation, rather than having to do a bind mount
> followed by a remount, which is fiddly and can block for non-trivial
> periods of time (on sb->s_umount?).
>
> For recursive bind mounts, only the root of the tree being bound
> inherits the per-mount flags from the mount() arguments; sub-mounts
> inherit their per-mount flags from the source tree as usual.

I think this concept is reasonable, but I don't think MS_BIND_FLAGS
is a descriptive name for this flag.  MS_EXPLICIT_FLAGS might be better
but still isn't optimal.

> +static struct vfsmount *
> +__copy_tree(struct vfsmount *mnt, struct dentry *dentry,
> +	    int flag, int mnt_flags)

> +struct vfsmount *copy_tree(struct vfsmount *mnt, struct dentry *dentry,
> +			   int flag)
> +{
> +	return __copy_tree(mnt, dentry, flag, mnt->mnt_flags);
> +}

Please just change copy_tree to have an additional parameter.  There's
just four callers of it in the tree, and three of them want the new
parameter.

> +	/* Use the source mount flags unless the user passed MS_BIND_FLAGS */

I think this comment could be made a little more explicit.

	/*
	 * If MS_EXPLICIT_FLAGS is passed in we will take the paramters
	 * the user has specified.  The default behaviour for bind
	 * mounts is however to take the flags from existing mount
	 * instances for the same superblock.
	 */
	 
> #define MS_RELATIME	(1<<21)	/* Update atime relative to mtime/ctime. */
> #define MS_KERNMOUNT	(1<<22) /* this is a kern_mount call */
> #define MS_I_VERSION	(1<<23) /* Update inode I_version field */
> +#define MS_BIND_FLAGS	(1<<24) /* For MS_BIND, take mnt_flags from
> +				 * args, not from source mountpoint */
> #define MS_ACTIVE	(1<<30)
> #define MS_NOUSER	(1<<31)

this looks like whitespace damage to me.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/