| 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
| ||
|
Message-ID: <CAOQ4uxiorvXhhJbCsGo-B7aBX0BbSYp7wUHmYS1e1xxJ4dpF3w@mail.gmail.com>
Date: Mon, 31 Jan 2022 17:29:58 +0200
From: Amir Goldstein <amir73il@...il.com>
To: David Howells <dhowells@...hat.com>
Cc: Miklos Szeredi <miklos@...redi.hu>,
overlayfs <linux-unionfs@...r.kernel.org>,
linux-cachefs@...hat.com, Christoph Hellwig <hch@...radead.org>,
Jeff Layton <jlayton@...nel.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Linus Torvalds <torvalds@...ux-foundation.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into
something generic
On Mon, Jan 31, 2022 at 5:13 PM David Howells <dhowells@...hat.com> wrote:
>
> Turn overlayfs's I_OVL_INUSE into something generic that cachefiles can
> also use for excluding access to its own cache files by renaming it to
> I_EXCL_INUSE as suggested by Amir[1] and hoisting the helpers to generic
> code.
>
> Suggested-by: Amir Goldstein <amir73il@...il.com>
> Signed-off-by: David Howells <dhowells@...hat.com>
> cc: Amir Goldstein <amir73il@...il.com>
> cc: Miklos Szeredi <miklos@...redi.hu>
> cc: linux-unionfs@...r.kernel.org
> cc: linux-cachefs@...hat.com
> Link: https://lore.kernel.org/r/CAOQ4uxhRS3MGEnCUDcsB1RL0d1Oy0g0Rzm75hVFAJw2dJ7uKSA@mail.gmail.com/ [1]
> ---
>
> fs/inode.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> fs/overlayfs/overlayfs.h | 3 ---
> fs/overlayfs/super.c | 12 ++++++------
> fs/overlayfs/util.c | 43 -------------------------------------------
> include/linux/fs.h | 22 +++++++++++++++++++---
> 5 files changed, 68 insertions(+), 55 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 63324df6fa27..954719f66113 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2405,3 +2405,46 @@ struct timespec64 current_time(struct inode *inode)
> return timestamp_truncate(now, inode);
> }
> EXPORT_SYMBOL(current_time);
> +
> +/**
> + * inode_excl_inuse_trylock - Try to exclusively lock an inode for kernel access
> + * @dentry: Reference to the inode to be locked
> + *
> + * Try to gain exclusive access to an inode for a kernel service, returning
> + * true if successful.
> + */
> +bool inode_excl_inuse_trylock(struct dentry *dentry)
> +{
> + struct inode *inode = d_inode(dentry);
> + bool locked = false;
> +
> + spin_lock(&inode->i_lock);
> + if (!(inode->i_state & I_EXCL_INUSE)) {
> + inode->i_state |= I_EXCL_INUSE;
> + locked = true;
> + }
> + spin_unlock(&inode->i_lock);
> +
> + return locked;
> +}
> +EXPORT_SYMBOL(inode_excl_inuse_trylock);
> +
> +/**
> + * inode_excl_inuse_unlock - Unlock exclusive kernel access to an inode
> + * @dentry: Reference to the inode to be unlocked
> + *
> + * Drop exclusive access to an inode for a kernel service. A warning is given
> + * if the inode was not marked for exclusive access.
> + */
> +void inode_excl_inuse_unlock(struct dentry *dentry)
> +{
> + if (dentry) {
> + struct inode *inode = d_inode(dentry);
> +
> + spin_lock(&inode->i_lock);
> + WARN_ON(!(inode->i_state & I_EXCL_INUSE));
> + inode->i_state &= ~I_EXCL_INUSE;
> + spin_unlock(&inode->i_lock);
> + }
> +}
> +EXPORT_SYMBOL(inode_excl_inuse_unlock);
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 2cd5741c873b..8415c0c6d40c 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -337,9 +337,6 @@ int ovl_check_setxattr(struct ovl_fs *ofs, struct dentry *upperdentry,
> enum ovl_xattr ox, const void *value, size_t size,
> int xerr);
> int ovl_set_impure(struct dentry *dentry, struct dentry *upperdentry);
> -bool ovl_inuse_trylock(struct dentry *dentry);
> -void ovl_inuse_unlock(struct dentry *dentry);
> -bool ovl_is_inuse(struct dentry *dentry);
> bool ovl_need_index(struct dentry *dentry);
> int ovl_nlink_start(struct dentry *dentry);
> void ovl_nlink_end(struct dentry *dentry);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 7bb0a47cb615..5c3361a2dc7c 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -224,10 +224,10 @@ static void ovl_free_fs(struct ovl_fs *ofs)
> dput(ofs->indexdir);
> dput(ofs->workdir);
> if (ofs->workdir_locked)
> - ovl_inuse_unlock(ofs->workbasedir);
> + inode_excl_inuse_unlock(ofs->workbasedir);
> dput(ofs->workbasedir);
> if (ofs->upperdir_locked)
> - ovl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
> + inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
>
> /* Hack! Reuse ofs->layers as a vfsmount array before freeing it */
> mounts = (struct vfsmount **) ofs->layers;
> @@ -1239,7 +1239,7 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
> if (upper_mnt->mnt_sb->s_flags & SB_NOSEC)
> sb->s_flags |= SB_NOSEC;
>
> - if (ovl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
> + if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
> ofs->upperdir_locked = true;
> } else {
> err = ovl_report_in_use(ofs, "upperdir");
> @@ -1499,7 +1499,7 @@ static int ovl_get_workdir(struct super_block *sb, struct ovl_fs *ofs,
>
> ofs->workbasedir = dget(workpath.dentry);
>
> - if (ovl_inuse_trylock(ofs->workbasedir)) {
> + if (inode_excl_inuse_trylock(ofs->workbasedir)) {
> ofs->workdir_locked = true;
> } else {
> err = ovl_report_in_use(ofs, "workdir");
> @@ -1722,7 +1722,7 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
> if (err)
> goto out;
>
> - if (ovl_is_inuse(stack[i].dentry)) {
> + if (inode_is_excl_inuse(stack[i].dentry)) {
> err = ovl_report_in_use(ofs, "lowerdir");
> if (err) {
> iput(trap);
> @@ -1872,7 +1872,7 @@ static int ovl_check_layer(struct super_block *sb, struct ovl_fs *ofs,
> if (is_lower && ovl_lookup_trap_inode(sb, parent)) {
> err = -ELOOP;
> pr_err("overlapping %s path\n", name);
> - } else if (ovl_is_inuse(parent)) {
> + } else if (inode_is_excl_inuse(parent)) {
> err = ovl_report_in_use(ofs, name);
> }
> next = parent;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index f48284a2a896..748c4b22deb3 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -724,49 +724,6 @@ int ovl_set_protattr(struct inode *inode, struct dentry *upper,
> return 0;
> }
>
> -/**
> - * Caller must hold a reference to inode to prevent it from being freed while
> - * it is marked inuse.
> - */
> -bool ovl_inuse_trylock(struct dentry *dentry)
> -{
> - struct inode *inode = d_inode(dentry);
> - bool locked = false;
> -
> - spin_lock(&inode->i_lock);
> - if (!(inode->i_state & I_OVL_INUSE)) {
> - inode->i_state |= I_OVL_INUSE;
> - locked = true;
> - }
> - spin_unlock(&inode->i_lock);
> -
> - return locked;
> -}
> -
> -void ovl_inuse_unlock(struct dentry *dentry)
> -{
> - if (dentry) {
> - struct inode *inode = d_inode(dentry);
> -
> - spin_lock(&inode->i_lock);
> - WARN_ON(!(inode->i_state & I_OVL_INUSE));
> - inode->i_state &= ~I_OVL_INUSE;
> - spin_unlock(&inode->i_lock);
> - }
> -}
> -
> -bool ovl_is_inuse(struct dentry *dentry)
> -{
> - struct inode *inode = d_inode(dentry);
> - bool inuse;
> -
> - spin_lock(&inode->i_lock);
> - inuse = (inode->i_state & I_OVL_INUSE);
> - spin_unlock(&inode->i_lock);
> -
> - return inuse;
> -}
Please leave ovl_* as wrappers instead of changing super.c.
Thanks,
Amir.
Powered by blists - more mailing lists