* Re: [PATCH 12/13] ovl: remove ovl_lock_rename_workdir()
From: Jeff Layton @ 2026-02-05 12:58 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-13-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> This function is unused.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/overlayfs/overlayfs.h | 2 --
> fs/overlayfs/util.c | 25 -------------------------
> 2 files changed, 27 deletions(-)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 4fb4750a83e4..3eedc2684c23 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -569,8 +569,6 @@ 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);
> -int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
> - struct dentry *upperdir, struct dentry *upper);
> int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path,
> struct ovl_metacopy *data);
> int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d,
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 94986d11a166..810c8752b4f7 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -1213,31 +1213,6 @@ void ovl_nlink_end(struct dentry *dentry)
> ovl_inode_unlock(inode);
> }
>
> -int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
> - struct dentry *upperdir, struct dentry *upper)
> -{
> - struct dentry *trap;
> -
> - /* Workdir should not be subdir of upperdir and vice versa */
> - trap = lock_rename(workdir, upperdir);
> - if (IS_ERR(trap))
> - goto err;
> - if (trap)
> - goto err_unlock;
> - if (work && (work->d_parent != workdir || d_unhashed(work)))
> - goto err_unlock;
> - if (upper && (upper->d_parent != upperdir || d_unhashed(upper)))
> - goto err_unlock;
> -
> - return 0;
> -
> -err_unlock:
> - unlock_rename(workdir, upperdir);
> -err:
> - pr_err("failed to lock workdir+upperdir\n");
> - return -EIO;
> -}
> -
> /*
> * err < 0, 0 if no metacopy xattr, metacopy data size if xattr found.
> * an empty xattr returns OVL_METACOPY_MIN_SIZE to distinguish from no xattr value.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 04/13] Apparmor: Use simple_start_creating() / simple_done_creating()
From: Jeff Layton @ 2026-02-05 12:58 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-5-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> Instead of explicitly locking the parent and performing a look up in
> apparmor, use simple_start_creating(), and then simple_done_creating()
> to unlock and drop the dentry.
>
> This removes the need to check for an existing entry (as
> simple_start_creating() acts like an exclusive create and can return
> -EEXIST), simplifies error paths, and keeps dir locking code
> centralised.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> security/apparmor/apparmorfs.c | 38 ++++++++--------------------------
> 1 file changed, 9 insertions(+), 29 deletions(-)
>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 907bd2667e28..7f78c36e6e50 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -282,32 +282,19 @@ static struct dentry *aafs_create(const char *name, umode_t mode,
>
> dir = d_inode(parent);
>
> - inode_lock(dir);
> - dentry = lookup_noperm(&QSTR(name), parent);
> + dentry = simple_start_creating(parent, name);
> if (IS_ERR(dentry)) {
> error = PTR_ERR(dentry);
> - goto fail_lock;
> - }
> -
> - if (d_really_is_positive(dentry)) {
> - error = -EEXIST;
> - goto fail_dentry;
> + goto fail;
> }
>
> error = __aafs_setup_d_inode(dir, dentry, mode, data, link, fops, iops);
> + simple_done_creating(dentry);
> if (error)
> - goto fail_dentry;
> - inode_unlock(dir);
> -
> - return dentry;
> -
> -fail_dentry:
> - dput(dentry);
> -
> -fail_lock:
> - inode_unlock(dir);
> + goto fail;
> + return 0;
As KTR points out, this should be "return NULL;"
> +fail:
> simple_release_fs(&aafs_mnt, &aafs_count);
> -
> return ERR_PTR(error);
> }
>
> @@ -2572,8 +2559,7 @@ static int aa_mk_null_file(struct dentry *parent)
> if (error)
> return error;
>
> - inode_lock(d_inode(parent));
> - dentry = lookup_noperm(&QSTR(NULL_FILE_NAME), parent);
> + dentry = simple_start_creating(parent, NULL_FILE_NAME);
> if (IS_ERR(dentry)) {
> error = PTR_ERR(dentry);
> goto out;
> @@ -2581,7 +2567,7 @@ static int aa_mk_null_file(struct dentry *parent)
> inode = new_inode(parent->d_inode->i_sb);
> if (!inode) {
> error = -ENOMEM;
> - goto out1;
> + goto out;
> }
>
> inode->i_ino = get_next_ino();
> @@ -2593,18 +2579,12 @@ static int aa_mk_null_file(struct dentry *parent)
> aa_null.dentry = dget(dentry);
> aa_null.mnt = mntget(mount);
>
> - error = 0;
> -
> -out1:
> - dput(dentry);
> out:
> - inode_unlock(d_inode(parent));
> + simple_done_creating(dentry);
> simple_release_fs(&mount, &count);
> return error;
> }
>
> -
> -
> static const char *policy_get_link(struct dentry *dentry,
> struct inode *inode,
> struct delayed_call *done)
Assuming you fix the minor problem above.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 13/13] VFS: unexport lock_rename(), lock_rename_child(), unlock_rename()
From: Jeff Layton @ 2026-02-05 12:41 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-14-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> These three function are now only used in namei.c, so they don't need to
> be exported.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> Documentation/filesystems/porting.rst | 7 +++++++
> fs/namei.c | 9 +++------
> include/linux/namei.h | 3 ---
> 3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> index ed86c95d9d01..5f7008172f14 100644
> --- a/Documentation/filesystems/porting.rst
> +++ b/Documentation/filesystems/porting.rst
> @@ -1347,3 +1347,10 @@ implementation should set it to generic_setlease().
>
> lookup_one_qstr_excl() is no longer exported - use start_creating() or
> similar.
> +---
> +
> +** mandatory**
> +
> +lock_rename(), lock_rename_child(), unlock_rename() are no
> +longer available. Use start_renaming() or similar.
> +
> diff --git a/fs/namei.c b/fs/namei.c
> index 307b4d0866b8..0bc82bf90adc 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3713,7 +3713,7 @@ static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2)
> /*
> * p1 and p2 should be directories on the same fs.
> */
> -struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
> +static struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
> {
> if (p1 == p2) {
> inode_lock_nested(p1->d_inode, I_MUTEX_PARENT);
> @@ -3723,12 +3723,11 @@ struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
> mutex_lock(&p1->d_sb->s_vfs_rename_mutex);
> return lock_two_directories(p1, p2);
> }
> -EXPORT_SYMBOL(lock_rename);
>
> /*
> * c1 and p2 should be on the same fs.
> */
> -struct dentry *lock_rename_child(struct dentry *c1, struct dentry *p2)
> +static struct dentry *lock_rename_child(struct dentry *c1, struct dentry *p2)
> {
> if (READ_ONCE(c1->d_parent) == p2) {
> /*
> @@ -3765,9 +3764,8 @@ struct dentry *lock_rename_child(struct dentry *c1, struct dentry *p2)
> mutex_unlock(&c1->d_sb->s_vfs_rename_mutex);
> return NULL;
> }
> -EXPORT_SYMBOL(lock_rename_child);
>
> -void unlock_rename(struct dentry *p1, struct dentry *p2)
> +static void unlock_rename(struct dentry *p1, struct dentry *p2)
> {
> inode_unlock(p1->d_inode);
> if (p1 != p2) {
> @@ -3775,7 +3773,6 @@ void unlock_rename(struct dentry *p1, struct dentry *p2)
> mutex_unlock(&p1->d_sb->s_vfs_rename_mutex);
> }
> }
> -EXPORT_SYMBOL(unlock_rename);
>
> /**
> * __start_renaming - lookup and lock names for rename
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index c7a7288cdd25..2ad6dd9987b9 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -165,9 +165,6 @@ extern int follow_down_one(struct path *);
> extern int follow_down(struct path *path, unsigned int flags);
> extern int follow_up(struct path *);
>
> -extern struct dentry *lock_rename(struct dentry *, struct dentry *);
> -extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
> -extern void unlock_rename(struct dentry *, struct dentry *);
> int start_renaming(struct renamedata *rd, int lookup_flags,
> struct qstr *old_last, struct qstr *new_last);
> int start_renaming_dentry(struct renamedata *rd, int lookup_flags,
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 12/13] ovl: remove ovl_lock_rename_workdir()
From: Jeff Layton @ 2026-02-05 12:40 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-13-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> This function is unused.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/overlayfs/overlayfs.h | 2 --
> fs/overlayfs/util.c | 25 -------------------------
> 2 files changed, 27 deletions(-)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 4fb4750a83e4..3eedc2684c23 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -569,8 +569,6 @@ 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);
> -int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
> - struct dentry *upperdir, struct dentry *upper);
> int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path,
> struct ovl_metacopy *data);
> int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d,
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 94986d11a166..810c8752b4f7 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -1213,31 +1213,6 @@ void ovl_nlink_end(struct dentry *dentry)
> ovl_inode_unlock(inode);
> }
>
> -int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work,
> - struct dentry *upperdir, struct dentry *upper)
> -{
> - struct dentry *trap;
> -
> - /* Workdir should not be subdir of upperdir and vice versa */
> - trap = lock_rename(workdir, upperdir);
> - if (IS_ERR(trap))
> - goto err;
> - if (trap)
> - goto err_unlock;
> - if (work && (work->d_parent != workdir || d_unhashed(work)))
> - goto err_unlock;
> - if (upper && (upper->d_parent != upperdir || d_unhashed(upper)))
> - goto err_unlock;
> -
> - return 0;
> -
> -err_unlock:
> - unlock_rename(workdir, upperdir);
> -err:
> - pr_err("failed to lock workdir+upperdir\n");
> - return -EIO;
> -}
> -
> /*
> * err < 0, 0 if no metacopy xattr, metacopy data size if xattr found.
> * an empty xattr returns OVL_METACOPY_MIN_SIZE to distinguish from no xattr value.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 11/13] ovl: use is_subdir() for testing if one thing is a subdir of another
From: Jeff Layton @ 2026-02-05 12:38 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-12-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> Rather than using lock_rename(), use the more obvious is_subdir() for
> ensuring that neither upper nor workdir contain the other.
> Also be explicit in the comment that the two directories cannot be the
> same.
>
> As this is a point-it-time sanity check and does not provide any
> on-going guarantees, the removal of locking does not introduce any
> interesting races.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/overlayfs/super.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index ba9146f22a2c..2fd3e0aee50e 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -451,18 +451,13 @@ static int ovl_lower_dir(const char *name, const struct path *path,
> return 0;
> }
>
> -/* Workdir should not be subdir of upperdir and vice versa */
> +/*
> + * Workdir should not be subdir of upperdir and vice versa, and
> + * they should not be the same.
> + */
> static bool ovl_workdir_ok(struct dentry *workdir, struct dentry *upperdir)
> {
> - bool ok = false;
> -
> - if (workdir != upperdir) {
> - struct dentry *trap = lock_rename(workdir, upperdir);
> - if (!IS_ERR(trap))
> - unlock_rename(workdir, upperdir);
> - ok = (trap == NULL);
> - }
> - return ok;
> + return !is_subdir(workdir, upperdir) && !is_subdir(upperdir, workdir);
> }
>
> static int ovl_setup_trap(struct super_block *sb, struct dentry *dir,
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 09/13] cachefiles: change cachefiles_bury_object to use start_renaming_dentry()
From: Jeff Layton @ 2026-02-05 12:38 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-10-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> Rather then using lock_rename() and lookup_one() etc we can use
> the new start_renaming_dentry(). This is part of centralising dir
> locking and lookup so that locking rules can be changed.
>
> Some error check are removed as not necessary. Checks for rep being a
> non-dir or IS_DEADDIR and the check that ->graveyard is still a
> directory only provide slightly more informative errors and have been
> dropped.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/cachefiles/namei.c | 76 ++++++++-----------------------------------
> 1 file changed, 14 insertions(+), 62 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index e5ec90dccc27..3af42ec78411 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -270,7 +270,8 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> struct dentry *rep,
> enum fscache_why_object_killed why)
> {
> - struct dentry *grave, *trap;
> + struct dentry *grave;
> + struct renamedata rd = {};
> struct path path, path_to_graveyard;
> char nbuffer[8 + 8 + 1];
> int ret;
> @@ -302,77 +303,36 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> (uint32_t) ktime_get_real_seconds(),
> (uint32_t) atomic_inc_return(&cache->gravecounter));
>
> - /* do the multiway lock magic */
> - trap = lock_rename(cache->graveyard, dir);
> - if (IS_ERR(trap))
> - return PTR_ERR(trap);
> -
> - /* do some checks before getting the grave dentry */
> - if (rep->d_parent != dir || IS_DEADDIR(d_inode(rep))) {
> - /* the entry was probably culled when we dropped the parent dir
> - * lock */
> - unlock_rename(cache->graveyard, dir);
> - _leave(" = 0 [culled?]");
> - return 0;
> - }
> -
> - if (!d_can_lookup(cache->graveyard)) {
> - unlock_rename(cache->graveyard, dir);
> - cachefiles_io_error(cache, "Graveyard no longer a directory");
> - return -EIO;
> - }
> -
> - if (trap == rep) {
> - unlock_rename(cache->graveyard, dir);
> - cachefiles_io_error(cache, "May not make directory loop");
> + rd.mnt_idmap = &nop_mnt_idmap;
> + rd.old_parent = dir;
> + rd.new_parent = cache->graveyard;
> + rd.flags = 0;
> + ret = start_renaming_dentry(&rd, 0, rep, &QSTR(nbuffer));
> + if (ret) {
> + cachefiles_io_error(cache, "Cannot lock/lookup in graveyard");
> return -EIO;
> }
>
> if (d_mountpoint(rep)) {
> - unlock_rename(cache->graveyard, dir);
> + end_renaming(&rd);
> cachefiles_io_error(cache, "Mountpoint in cache");
> return -EIO;
> }
>
> - grave = lookup_one(&nop_mnt_idmap, &QSTR(nbuffer), cache->graveyard);
> - if (IS_ERR(grave)) {
> - unlock_rename(cache->graveyard, dir);
> - trace_cachefiles_vfs_error(object, d_inode(cache->graveyard),
> - PTR_ERR(grave),
> - cachefiles_trace_lookup_error);
> -
> - if (PTR_ERR(grave) == -ENOMEM) {
> - _leave(" = -ENOMEM");
> - return -ENOMEM;
> - }
> -
> - cachefiles_io_error(cache, "Lookup error %ld", PTR_ERR(grave));
> - return -EIO;
> - }
> -
> + grave = rd.new_dentry;
> if (d_is_positive(grave)) {
> - unlock_rename(cache->graveyard, dir);
> - dput(grave);
> + end_renaming(&rd);
> grave = NULL;
> cond_resched();
> goto try_again;
> }
>
> if (d_mountpoint(grave)) {
> - unlock_rename(cache->graveyard, dir);
> - dput(grave);
> + end_renaming(&rd);
> cachefiles_io_error(cache, "Mountpoint in graveyard");
> return -EIO;
> }
>
> - /* target should not be an ancestor of source */
> - if (trap == grave) {
> - unlock_rename(cache->graveyard, dir);
> - dput(grave);
> - cachefiles_io_error(cache, "May not make directory loop");
> - return -EIO;
> - }
> -
> /* attempt the rename */
> path.mnt = cache->mnt;
> path.dentry = dir;
> @@ -382,13 +342,6 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> if (ret < 0) {
> cachefiles_io_error(cache, "Rename security error %d", ret);
> } else {
> - struct renamedata rd = {
> - .mnt_idmap = &nop_mnt_idmap,
> - .old_parent = dir,
> - .old_dentry = rep,
> - .new_parent = cache->graveyard,
> - .new_dentry = grave,
> - };
> trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
> ret = cachefiles_inject_read_error();
> if (ret == 0)
> @@ -402,8 +355,7 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
> }
>
> __cachefiles_unmark_inode_in_use(object, d_inode(rep));
> - unlock_rename(cache->graveyard, dir);
> - dput(grave);
> + end_renaming(&rd);
> _leave(" = 0");
> return 0;
> }
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 08/13] ovl: Simplify ovl_lookup_real_one()
From: Jeff Layton @ 2026-02-05 12:38 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-9-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> The primary purpose of this patch is to remove the locking from
> ovl_lookup_real_one() as part of centralising all locking of directories
> for name operations.
>
> The locking here isn't needed. By performing consistency tests after
> the lookup we can be sure that the result of the lookup was valid at
> least for a moment, which is all the original code promised.
>
> lookup_noperm_unlocked() is used for the lookup and it will take the
> lock if needed only where it is needed.
>
> Also:
> - don't take a reference to real->d_parent. The parent is
> only use for a pointer comparison, and no reference is needed for
> that.
> - Several "if" statements have a "goto" followed by "else" - the
> else isn't needed: the following statement can directly follow
> the "if" as a new statement
> - Use a consistent pattern of setting "err" before performing a test
> and possibly going to "fail".
> - remove the "out" label (now that we don't need to dput(parent) or
> unlock) and simply return from fail:.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/overlayfs/export.c | 61 ++++++++++++++++++-------------------------
> 1 file changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
> index 83f80fdb1567..dcd28ffc4705 100644
> --- a/fs/overlayfs/export.c
> +++ b/fs/overlayfs/export.c
> @@ -359,59 +359,50 @@ static struct dentry *ovl_lookup_real_one(struct dentry *connected,
> struct dentry *real,
> const struct ovl_layer *layer)
> {
> - struct inode *dir = d_inode(connected);
> - struct dentry *this, *parent = NULL;
> + struct dentry *this;
> struct name_snapshot name;
> int err;
>
> /*
> - * Lookup child overlay dentry by real name. The dir mutex protects us
> - * from racing with overlay rename. If the overlay dentry that is above
> - * real has already been moved to a parent that is not under the
> - * connected overlay dir, we return -ECHILD and restart the lookup of
> - * connected real path from the top.
> + * @connected is a directory in the overlay and @real is an object
> + * on @layer which is expected to be a child of @connected.
> + * The goal is to return a dentry from the overlay which corresponds
> + * to @real. This is done by looking up the name from @real in
> + * @connected and checking that the result meets expectations.
> + *
> + * Return %-ECHILD if the parent of @real no-longer corresponds to
> + * @connected, and %-ESTALE if the dentry found by lookup doesn't
> + * correspond to @real.
> */
> - inode_lock_nested(dir, I_MUTEX_PARENT);
> - err = -ECHILD;
> - parent = dget_parent(real);
> - if (ovl_dentry_real_at(connected, layer->idx) != parent)
> - goto fail;
>
> - /*
> - * We also need to take a snapshot of real dentry name to protect us
> - * from racing with underlying layer rename. In this case, we don't
> - * care about returning ESTALE, only from dereferencing a free name
> - * pointer because we hold no lock on the real dentry.
> - */
> take_dentry_name_snapshot(&name, real);
> - /*
> - * No idmap handling here: it's an internal lookup.
> - */
> - this = lookup_noperm(&name.name, connected);
> + this = lookup_noperm_unlocked(&name.name, connected);
> release_dentry_name_snapshot(&name);
> +
> + err = -ECHILD;
> + if (ovl_dentry_real_at(connected, layer->idx) != real->d_parent)
> + goto fail;
> +
> err = PTR_ERR(this);
> - if (IS_ERR(this)) {
> + if (IS_ERR(this))
> goto fail;
> - } else if (!this || !this->d_inode) {
> - dput(this);
> - err = -ENOENT;
> +
> + err = -ENOENT;
> + if (!this || !this->d_inode)
> goto fail;
> - } else if (ovl_dentry_real_at(this, layer->idx) != real) {
> - dput(this);
> - err = -ESTALE;
> +
> + err = -ESTALE;
> + if (ovl_dentry_real_at(this, layer->idx) != real)
> goto fail;
> - }
>
> -out:
> - dput(parent);
> - inode_unlock(dir);
> return this;
>
> fail:
> pr_warn_ratelimited("failed to lookup one by real (%pd2, layer=%d, connected=%pd2, err=%i)\n",
> real, layer->idx, connected, err);
> - this = ERR_PTR(err);
> - goto out;
> + if (!IS_ERR(this))
> + dput(this);
> + return ERR_PTR(err);
> }
>
> static struct dentry *ovl_lookup_real(struct super_block *sb,
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 07/13] VFS: make lookup_one_qstr_excl() static.
From: Jeff Layton @ 2026-02-05 12:37 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-8-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> lookup_one_qstr_excl() is no longer used outside of namei.c, so
> make it static.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> Documentation/filesystems/porting.rst | 7 +++++++
> fs/namei.c | 5 ++---
> include/linux/namei.h | 3 ---
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
> index ed3ac56e3c76..ed86c95d9d01 100644
> --- a/Documentation/filesystems/porting.rst
> +++ b/Documentation/filesystems/porting.rst
> @@ -1340,3 +1340,10 @@ The ->setlease() file_operation must now be explicitly set in order to provide
> support for leases. When set to NULL, the kernel will now return -EINVAL to
> attempts to set a lease. Filesystems that wish to use the kernel-internal lease
> implementation should set it to generic_setlease().
> +
> +---
> +
> +**mandatory**
> +
> +lookup_one_qstr_excl() is no longer exported - use start_creating() or
> +similar.
> diff --git a/fs/namei.c b/fs/namei.c
> index 40af78ddfb1b..307b4d0866b8 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1730,8 +1730,8 @@ static struct dentry *lookup_dcache(const struct qstr *name,
> * Will return -ENOENT if name isn't found and LOOKUP_CREATE wasn't passed.
> * Will return -EEXIST if name is found and LOOKUP_EXCL was passed.
> */
> -struct dentry *lookup_one_qstr_excl(const struct qstr *name,
> - struct dentry *base, unsigned int flags)
> +static struct dentry *lookup_one_qstr_excl(const struct qstr *name,
> + struct dentry *base, unsigned int flags)
> {
> struct dentry *dentry;
> struct dentry *old;
> @@ -1768,7 +1768,6 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
> }
> return dentry;
> }
> -EXPORT_SYMBOL(lookup_one_qstr_excl);
>
> /**
> * lookup_fast - do fast lockless (but racy) lookup of a dentry
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 58600cf234bc..c7a7288cdd25 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -54,9 +54,6 @@ extern int path_pts(struct path *path);
>
> extern int user_path_at(int, const char __user *, unsigned, struct path *);
>
> -struct dentry *lookup_one_qstr_excl(const struct qstr *name,
> - struct dentry *base,
> - unsigned int flags);
> extern int kern_path(const char *, unsigned, struct path *);
> struct dentry *kern_path_parent(const char *name, struct path *parent);
>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 06/13] nfsd: switch purge_old() to use start_removing_noperm()
From: Jeff Layton @ 2026-02-05 12:36 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-7-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> Rather than explicit locking, use the start_removing_noperm() and
> end_removing() wrappers.
> This was not done with other start_removing changes due to conflicting
> in-flight patches.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/nfsd/nfs4recover.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 441dfbfe2d2b..52fbe723a3c8 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -351,16 +351,14 @@ purge_old(struct dentry *parent, char *cname, struct nfsd_net *nn)
> if (nfs4_has_reclaimed_state(name, nn))
> goto out_free;
>
> - inode_lock_nested(d_inode(parent), I_MUTEX_PARENT);
> - child = lookup_one(&nop_mnt_idmap, &QSTR(cname), parent);
> + child = start_removing_noperm(parent, &QSTR(cname));
> if (!IS_ERR(child)) {
> status = vfs_rmdir(&nop_mnt_idmap, d_inode(parent), child, NULL);
> if (status)
> printk("failed to remove client recovery directory %pd\n",
> child);
> - dput(child);
> }
> - inode_unlock(d_inode(parent));
> + end_removing(child);
>
> out_free:
> kfree(name.data);
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 05/13] selinux: Use simple_start_creating() / simple_done_creating()
From: Jeff Layton @ 2026-02-05 12:36 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-6-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> Instead of explicitly locking the parent and performing a lookup in
> selinux, use simple_start_creating(), and then use
> simple_done_creating() to unlock.
>
> This extends the region that the directory is locked for, and also
> performs a lookup.
> The lock extension is of no real consequence.
> The lookup uses simple_lookup() and so always succeeds. Thus when
> d_make_persistent() is called the dentry will already be hashed.
> d_make_persistent() handles this case.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> security/selinux/selinuxfs.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 896acad1f5f7..97e02cd5a9dc 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -1930,15 +1930,16 @@ static const struct inode_operations swapover_dir_inode_operations = {
> static struct dentry *sel_make_swapover_dir(struct super_block *sb,
> unsigned long *ino)
> {
> - struct dentry *dentry = d_alloc_name(sb->s_root, ".swapover");
> + struct dentry *dentry;
> struct inode *inode;
>
> - if (!dentry)
> + inode = sel_make_inode(sb, S_IFDIR);
> + if (!inode)
> return ERR_PTR(-ENOMEM);
>
> - inode = sel_make_inode(sb, S_IFDIR);
> - if (!inode) {
> - dput(dentry);
> + dentry = simple_start_creating(sb->s_root, ".swapover");
> + if (!dentry) {
> + iput(inode);
> return ERR_PTR(-ENOMEM);
> }
>
> @@ -1946,11 +1947,9 @@ static struct dentry *sel_make_swapover_dir(struct super_block *sb,
> inode->i_ino = ++(*ino);
> /* directory inodes start off with i_nlink == 2 (for "." entry) */
> inc_nlink(inode);
> - inode_lock(sb->s_root->d_inode);
> d_make_persistent(dentry, inode);
> inc_nlink(sb->s_root->d_inode);
> - inode_unlock(sb->s_root->d_inode);
> - dput(dentry);
> + simple_done_creating(dentry);
> return dentry; // borrowed
> }
>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 02/13] VFS: move the start_dirop() kerndoc comment to before start_dirop()
From: Jeff Layton @ 2026-02-05 12:33 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-3-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> This kerneldoc comment was always meant for start_dirop(), not for
> __start_dirop() which is a static function and doesn't need
> documentation.
>
> It was in the wrong place and was then incorrectly renamed (instead of
> moved) and useless "documentation" was added for "@state" was provided.
>
> This patch reverts the name, removes the mention of @state, and moves
> the comment to where it belongs.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/namei.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index b28ecb699f32..40af78ddfb1b 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2841,20 +2841,6 @@ static int filename_parentat(int dfd, struct filename *name,
> return __filename_parentat(dfd, name, flags, parent, last, type, NULL);
> }
>
> -/**
> - * __start_dirop - begin a create or remove dirop, performing locking and lookup
> - * @parent: the dentry of the parent in which the operation will occur
> - * @name: a qstr holding the name within that parent
> - * @lookup_flags: intent and other lookup flags.
> - * @state: task state bitmask
> - *
> - * The lookup is performed and necessary locks are taken so that, on success,
> - * the returned dentry can be operated on safely.
> - * The qstr must already have the hash value calculated.
> - *
> - * Returns: a locked dentry, or an error.
> - *
> - */
> static struct dentry *__start_dirop(struct dentry *parent, struct qstr *name,
> unsigned int lookup_flags,
> unsigned int state)
> @@ -2876,6 +2862,19 @@ static struct dentry *__start_dirop(struct dentry *parent, struct qstr *name,
> return dentry;
> }
>
> +/**
> + * start_dirop - begin a create or remove dirop, performing locking and lookup
> + * @parent: the dentry of the parent in which the operation will occur
> + * @name: a qstr holding the name within that parent
> + * @lookup_flags: intent and other lookup flags.
> + *
> + * The lookup is performed and necessary locks are taken so that, on success,
> + * the returned dentry can be operated on safely.
> + * The qstr must already have the hash value calculated.
> + *
> + * Returns: a locked dentry, or an error.
> + *
> + */
> struct dentry *start_dirop(struct dentry *parent, struct qstr *name,
> unsigned int lookup_flags)
> {
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 01/13] fs/proc: Don't lock root inode when creating "self" and "thread-self"
From: Jeff Layton @ 2026-02-05 12:33 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-2-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> proc_setup_self() and proc_setup_thread_self() are only called from
> proc_fill_super() which is before the filesystem is "live". So there is
> no need to lock the root directory when adding "self" and "thread-self".
> This is clear from simple_fill_super() which provides similar
> functionality for other filesystems and does not lock anything.
>
> The locking is not harmful, except that it may be confusing to a reader.
> As part of a an effort to centralise all locking for directories for
> name-based operations (prior to changing some locking rules), it is
> simplest to remove the locking here.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/proc/self.c | 3 ---
> fs/proc/thread_self.c | 3 ---
> 2 files changed, 6 deletions(-)
>
> diff --git a/fs/proc/self.c b/fs/proc/self.c
> index 62d2c0cfe35c..56adf1c68f7a 100644
> --- a/fs/proc/self.c
> +++ b/fs/proc/self.c
> @@ -35,11 +35,9 @@ unsigned self_inum __ro_after_init;
>
> int proc_setup_self(struct super_block *s)
> {
> - struct inode *root_inode = d_inode(s->s_root);
> struct dentry *self;
> int ret = -ENOMEM;
>
> - inode_lock(root_inode);
> self = d_alloc_name(s->s_root, "self");
> if (self) {
> struct inode *inode = new_inode(s);
> @@ -55,7 +53,6 @@ int proc_setup_self(struct super_block *s)
> }
> dput(self);
> }
> - inode_unlock(root_inode);
>
> if (ret)
> pr_err("proc_fill_super: can't allocate /proc/self\n");
> diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
> index d6113dbe58e0..61ac62c3fd9f 100644
> --- a/fs/proc/thread_self.c
> +++ b/fs/proc/thread_self.c
> @@ -35,11 +35,9 @@ unsigned thread_self_inum __ro_after_init;
>
> int proc_setup_thread_self(struct super_block *s)
> {
> - struct inode *root_inode = d_inode(s->s_root);
> struct dentry *thread_self;
> int ret = -ENOMEM;
>
> - inode_lock(root_inode);
> thread_self = d_alloc_name(s->s_root, "thread-self");
> if (thread_self) {
> struct inode *inode = new_inode(s);
> @@ -55,7 +53,6 @@ int proc_setup_thread_self(struct super_block *s)
> }
> dput(thread_self);
> }
> - inode_unlock(root_inode);
>
> if (ret)
> pr_err("proc_fill_super: can't allocate /proc/thread-self\n");
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: David Hildenbrand (Arm) @ 2026-02-05 12:30 UTC (permalink / raw)
To: Lorenzo Stoakes, Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module, linux-mm,
rust-for-linux
In-Reply-To: <4c399644-97d3-40a3-a596-e4c93b713bdd@lucifer.local>
On 2/5/26 13:24, Lorenzo Stoakes wrote:
> On Thu, Feb 05, 2026 at 12:19:22PM +0000, Alice Ryhl wrote:
>> On Thu, Feb 05, 2026 at 01:13:57PM +0100, David Hildenbrand (arm) wrote:
>>>
>>> Could Rust just use zap_vma_ptes() or does it want to zap things in VMAs
>>> that are not VM_PFNMAP?
>>
>> The VMA is VM_MIXEDMAP, not VM_PFNMAP.
>
> OK this smells like David's cleanup could extend it to allow for
> VM_MIXEDMAP :) then we solve the export problem.
My thinking ... and while at it, gonna remove these functions to make
them a bit more ... consistent in naming.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Lorenzo Stoakes @ 2026-02-05 12:28 UTC (permalink / raw)
To: Miguel Ojeda
Cc: David Hildenbrand (arm), Alice Ryhl, Greg Kroah-Hartman,
Carlos Llamas, Alexander Viro, Christian Brauner, Jan Kara,
Paul Moore, James Morris, Serge E. Hallyn, Andrew Morton,
Dave Chinner, Qi Zheng, Roman Gushchin, Muchun Song,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux,
Christoph Hellwig
In-Reply-To: <CANiq72=ybFtqsh18zkC3e1iyR-RoffcL_ZDr-fU7SjzJiFERHw@mail.gmail.com>
On Thu, Feb 05, 2026 at 01:24:09PM +0100, Miguel Ojeda wrote:
> On Thu, Feb 5, 2026 at 12:58 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > What??
> >
> > Alice - can you confirm rust isn't exporting stuff that isn't explicitly marked
> > EXPORT_SYMBOL*() for use by other rust modules?
> >
> > It's important we keep this in sync, otherwise rust is overriding kernel policy.
>
> Currently, Rust GPL-exports every mangled symbol from the `kernel`
> crate. To call something you would need to be a Rust caller (not C --
> that is not supported at all, even if technically you could hack
> something up) and the Rust API would then need to give you access to
> it (i.e. you need to be able to pass the Rust language rules, e.g.
> being public etc.).
>
> In this case if we are talking about the `VmaRef` type, someone that
> can get a reference to a value of that type could then call the
> `zap_page_range_single` method. That in turns would try to call the C
> one, but that one is not exported, right? So it should be fine.
>
OK cool. Thanks for the explanation.
> In the future, for Rust, we may specify whether a particular crate
> exports or not (and perhaps even allow to export non-GPL, but
> originally it was decided to only export GPL stuff to be on the safe
> side; and perhaps in certain namespaces etc.).
We'd definitely need to keep this in sync with C exports, maybe we can find
a nice way of doing/checking this.
Generally we're _very_ conservative about exports, and actually it'd be
nice if maybe rust only provided GPL ones. The GPL is a good thing :) But I
guess we can figure that out later.
>
> Cheers,
> Miguel
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 10/13] ovl: change ovl_create_real() to get a new lock when re-opening created file.
From: Amir Goldstein @ 2026-02-05 12:26 UTC (permalink / raw)
To: NeilBrown
Cc: Christian Brauner, Alexander Viro, David Howells, Jan Kara,
Chuck Lever, Jeff Layton, Miklos Szeredi, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-11-neilb@ownmail.net>
On Wed, Feb 4, 2026 at 6:09 AM NeilBrown <neilb@ownmail.net> wrote:
>
> From: NeilBrown <neil@brown.name>
>
> When ovl_create_real() is used to create a file on the upper filesystem
> it needs to return the resulting dentry - positive and hashed.
> It is usually the case the that dentry passed to the create function
> (e.g. vfs_create()) will be suitable but this is not guaranteed. The
> filesystem may unhash that dentry forcing a repeat lookup next time the
> name is wanted.
>
> So ovl_create_real() must be (and is) aware of this and prepared to
> perform that lookup to get a hash positive dentry.
>
> This is currently done under that same directory lock that provided
> exclusion for the create. Proposed changes to locking will make this
> not possible - as the name, rather than the directory, will be locked.
> The new APIs provided for lookup and locking do not and cannot support
> this pattern.
>
> The lock isn't needed. ovl_create_real() can drop the lock and then get
> a new lock for the lookup - then check that the lookup returned the
> correct inode. In a well-behaved configuration where the upper
> filesystem is not being modified by a third party, this will always work
> reliably, and if there are separate modification it will fail cleanly.
>
> So change ovl_create_real() to drop the lock and call
> ovl_start_creating_upper() to find the correct dentry. Note that
> start_creating doesn't fail if the name already exists.
>
> This removes the only remaining use of ovl_lookup_upper, so it is
> removed.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/overlayfs/dir.c | 24 ++++++++++++++++++------
> fs/overlayfs/overlayfs.h | 7 -------
> 2 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index ff3dbd1ca61f..ec08904d084d 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -219,21 +219,33 @@ struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent,
> err = -EIO;
> } else if (d_unhashed(newdentry)) {
> struct dentry *d;
> + struct name_snapshot name;
> /*
> * Some filesystems (i.e. casefolded) may return an unhashed
> - * negative dentry from the ovl_lookup_upper() call before
> + * negative dentry from the ovl_start_creating_upper() call before
> * ovl_create_real().
According to the new locking rules, if the hashed dentry itself is
the synchronization object, is it going to be allowed to
filesystem to unhash the dentry while the dentry still in the
"creating" scope? It is hard for me to wrap my head around this.
Or do we need this here because some filesystems (casefold in
particular) are not going to support parallel creations?
> * In that case, lookup again after making the newdentry
> * positive, so ovl_create_upper() always returns a hashed
> * positive dentry.
> + * As we have to drop the lock before the lookup a race
> + * could result in a lookup failure. In that case we return
> + * an error.
> */
> - d = ovl_lookup_upper(ofs, newdentry->d_name.name, parent,
> - newdentry->d_name.len);
> - dput(newdentry);
> - if (IS_ERR_OR_NULL(d))
> + take_dentry_name_snapshot(&name, newdentry);
> + end_creating_keep(newdentry);
> + d = ovl_start_creating_upper(ofs, parent, &name.name);
> + release_dentry_name_snapshot(&name);
OK. not saying no to this (yet) but I have to admit that it is pretty
ugly that the callers of ovl_create_real() want to create a specific
stable name, which is could be passed in as const char *name
and yet we end up doing this weird dance here just to keep the name
from newdentry.
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Lorenzo Stoakes @ 2026-02-05 12:24 UTC (permalink / raw)
To: Alice Ryhl
Cc: David Hildenbrand (arm), Greg Kroah-Hartman, Carlos Llamas,
Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <aYSKyr7StGpGKNqW@google.com>
On Thu, Feb 05, 2026 at 12:19:22PM +0000, Alice Ryhl wrote:
> On Thu, Feb 05, 2026 at 01:13:57PM +0100, David Hildenbrand (arm) wrote:
> > On 2/5/26 13:10, Lorenzo Stoakes wrote:
> > > On Thu, Feb 05, 2026 at 11:58:00AM +0000, Alice Ryhl wrote:
> > > > On Thu, Feb 05, 2026 at 12:43:03PM +0100, David Hildenbrand (arm) wrote:
> > > > >
> > > > > I don't expect anybody to set zap_details, but yeah, it could be abused.
> > > > > It could be abused right now from anywhere else in the kernel
> > > > > where we don't build as a module :)
> > > > >
> > > > > Apparently we export a similar function in rust where we just removed the last parameter.
> > > >
> > > > To clarify, said Rust function gets inlined into Rust Binder, so Rust
> > > > Binder calls the zap_page_range_single() symbol directly.
> > >
> > > Presumably only for things compiled into the kernel right?
> >
> > Could Rust just use zap_vma_ptes() or does it want to zap things in VMAs
> > that are not VM_PFNMAP?
>
> The VMA is VM_MIXEDMAP, not VM_PFNMAP.
OK this smells like David's cleanup could extend it to allow for
VM_MIXEDMAP :) then we solve the export problem.
The two of these cause endless issues, it's really a mess...
>
> Alice
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Miguel Ojeda @ 2026-02-05 12:24 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: David Hildenbrand (arm), Alice Ryhl, Greg Kroah-Hartman,
Carlos Llamas, Alexander Viro, Christian Brauner, Jan Kara,
Paul Moore, James Morris, Serge E. Hallyn, Andrew Morton,
Dave Chinner, Qi Zheng, Roman Gushchin, Muchun Song,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module, linux-mm, rust-for-linux,
Christoph Hellwig
In-Reply-To: <ab63390c-9e75-4a45-9bf4-4ceb112ef07f@lucifer.local>
On Thu, Feb 5, 2026 at 12:58 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> What??
>
> Alice - can you confirm rust isn't exporting stuff that isn't explicitly marked
> EXPORT_SYMBOL*() for use by other rust modules?
>
> It's important we keep this in sync, otherwise rust is overriding kernel policy.
Currently, Rust GPL-exports every mangled symbol from the `kernel`
crate. To call something you would need to be a Rust caller (not C --
that is not supported at all, even if technically you could hack
something up) and the Rust API would then need to give you access to
it (i.e. you need to be able to pass the Rust language rules, e.g.
being public etc.).
In this case if we are talking about the `VmaRef` type, someone that
can get a reference to a value of that type could then call the
`zap_page_range_single` method. That in turns would try to call the C
one, but that one is not exported, right? So it should be fine.
In the future, for Rust, we may specify whether a particular crate
exports or not (and perhaps even allow to export non-GPL, but
originally it was decided to only export GPL stuff to be on the safe
side; and perhaps in certain namespaces etc.).
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH 03/13] libfs: change simple_done_creating() to use end_creating()
From: Jeff Layton @ 2026-02-05 12:19 UTC (permalink / raw)
To: NeilBrown, Christian Brauner, Alexander Viro, David Howells,
Jan Kara, Chuck Lever, Miklos Szeredi, Amir Goldstein,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley
Cc: linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-4-neilb@ownmail.net>
On Wed, 2026-02-04 at 15:57 +1100, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> simple_done_creating() and end_creating() are identical.
> So change the former to use the latter. This further centralises
> unlocking of directories.
>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/libfs.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index f1860dff86f2..db18b53fc189 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -2318,7 +2318,6 @@ EXPORT_SYMBOL(simple_start_creating);
> /* parent must have been held exclusive since simple_start_creating() */
> void simple_done_creating(struct dentry *child)
> {
> - inode_unlock(child->d_parent->d_inode);
> - dput(child);
> + end_creating(child);
> }
> EXPORT_SYMBOL(simple_done_creating);
nit: seems like it would be better to turn this into a static inline
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Alice Ryhl @ 2026-02-05 12:19 UTC (permalink / raw)
To: David Hildenbrand (arm)
Cc: Lorenzo Stoakes, Greg Kroah-Hartman, Carlos Llamas,
Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <8856c839-1a94-4e4d-9ded-d3b1627cd2cc@kernel.org>
On Thu, Feb 05, 2026 at 01:13:57PM +0100, David Hildenbrand (arm) wrote:
> On 2/5/26 13:10, Lorenzo Stoakes wrote:
> > On Thu, Feb 05, 2026 at 11:58:00AM +0000, Alice Ryhl wrote:
> > > On Thu, Feb 05, 2026 at 12:43:03PM +0100, David Hildenbrand (arm) wrote:
> > > >
> > > > I don't expect anybody to set zap_details, but yeah, it could be abused.
> > > > It could be abused right now from anywhere else in the kernel
> > > > where we don't build as a module :)
> > > >
> > > > Apparently we export a similar function in rust where we just removed the last parameter.
> > >
> > > To clarify, said Rust function gets inlined into Rust Binder, so Rust
> > > Binder calls the zap_page_range_single() symbol directly.
> >
> > Presumably only for things compiled into the kernel right?
>
> Could Rust just use zap_vma_ptes() or does it want to zap things in VMAs
> that are not VM_PFNMAP?
The VMA is VM_MIXEDMAP, not VM_PFNMAP.
Alice
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Lorenzo Stoakes @ 2026-02-05 12:18 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <aYSH5KG36fVQFePL@google.com>
On Thu, Feb 05, 2026 at 12:07:00PM +0000, Alice Ryhl wrote:
> On Thu, Feb 05, 2026 at 11:29:04AM +0000, Lorenzo Stoakes wrote:
> > We either need a wrapper that eliminates this parameter (but then we're adding a
> > wrapper to this behaviour that is literally for one driver that is _temporarily_
> > being modularised which is weak justifiction), or use of a function that invokes
> > it that is currently exported.
>
> I have not talked with distros about it, but quite a few of them enable
> Binder because one or two applications want to use Binder to emulate
> Android. I imagine that even if Android itself goes back to built-in,
> distros would want it as a module so that you don't have to load it for
> every user, rather than for the few users that want to use waydroid or
> similar.
>
> A few examples:
> https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/blob/5711a17344ec7cfd90443374a30d5cd3e9a9439e/config#L10993
> https://salsa.debian.org/kernel-team/linux/-/blob/debian/latest/debian/config/arm64/config?ref_type=heads#L106
> https://gitlab.com/cki-project/kernel-ark/-/blob/os-build/redhat/configs/fedora/generic/x86/CONFIG_ANDROID_BINDER_IPC?ref_type=heads
I mean you should update the cover letter to make this clear and drop the whole
reference to things being temporary, this is a lot more strident than the cover
letter is.
In any case, that has nothing to do with whether or not we export internal
implementation details to a module.
Something being in-tree compiled gets to use actually far too many internal
interfaces that really should not have been exposed, we've been far too leniant
about that, and that's something I want to address (mm has mm/*.h internal-only
headers, not sure how we'll deal with that with rust though).
Sadly even with in-tree, every interface you make available leads to driver
abuse. So something compiled in-tree using X, Y or Z interface doesn't mean that
it's correct or even wise, and modularising forces you to rethink that.
folio_mkclean() is a great example, we were about to be able to make that
mm-internal then 2 more filesystems started using it oops :)
>
> Alice
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Alice Ryhl @ 2026-02-05 12:16 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: David Hildenbrand (arm), Greg Kroah-Hartman, Carlos Llamas,
Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <e7247f3e-8a88-4b46-91ba-cb73cce1346a@lucifer.local>
On Thu, Feb 05, 2026 at 12:10:38PM +0000, Lorenzo Stoakes wrote:
> On Thu, Feb 05, 2026 at 11:58:00AM +0000, Alice Ryhl wrote:
> > On Thu, Feb 05, 2026 at 12:43:03PM +0100, David Hildenbrand (arm) wrote:
> > > On 2/5/26 12:29, Lorenzo Stoakes wrote:
> > > > On Thu, Feb 05, 2026 at 10:51:28AM +0000, Alice Ryhl wrote:
> > > > > bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
> > > > > {
> > > > > diff --git a/mm/memory.c b/mm/memory.c
> > > > > index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
> > > > > --- a/mm/memory.c
> > > > > +++ b/mm/memory.c
> > > > > @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
> > > > > zap_page_range_single_batched(&tlb, vma, address, size, details);
> > > > > tlb_finish_mmu(&tlb);
> > > > > }
> > > > > +EXPORT_SYMBOL(zap_page_range_single);
> > > >
> > > > Sorry but I don't want this exported at all.
> > > >
> > > > This is an internal implementation detail which allows fine-grained control of
> > > > behaviour via struct zap_details (which binder doesn't use, of course :)
> > >
> > > I don't expect anybody to set zap_details, but yeah, it could be abused.
> > > It could be abused right now from anywhere else in the kernel
> > > where we don't build as a module :)
> > >
> > > Apparently we export a similar function in rust where we just removed the last parameter.
> >
> > To clarify, said Rust function gets inlined into Rust Binder, so Rust
> > Binder calls the zap_page_range_single() symbol directly.
>
> Presumably only for things compiled into the kernel right?
No, building Rust Binder with =m triggers this error for me:
ERROR: modpost: "zap_page_range_single" [drivers/android/binder/rust_binder.ko] undefined!
Alice
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: David Hildenbrand (arm) @ 2026-02-05 12:13 UTC (permalink / raw)
To: Lorenzo Stoakes, Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module, linux-mm,
rust-for-linux
In-Reply-To: <e7247f3e-8a88-4b46-91ba-cb73cce1346a@lucifer.local>
On 2/5/26 13:10, Lorenzo Stoakes wrote:
> On Thu, Feb 05, 2026 at 11:58:00AM +0000, Alice Ryhl wrote:
>> On Thu, Feb 05, 2026 at 12:43:03PM +0100, David Hildenbrand (arm) wrote:
>>>
>>> I don't expect anybody to set zap_details, but yeah, it could be abused.
>>> It could be abused right now from anywhere else in the kernel
>>> where we don't build as a module :)
>>>
>>> Apparently we export a similar function in rust where we just removed the last parameter.
>>
>> To clarify, said Rust function gets inlined into Rust Binder, so Rust
>> Binder calls the zap_page_range_single() symbol directly.
>
> Presumably only for things compiled into the kernel right?
Could Rust just use zap_vma_ptes() or does it want to zap things in VMAs
that are not VM_PFNMAP?
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Lorenzo Stoakes @ 2026-02-05 12:12 UTC (permalink / raw)
To: David Hildenbrand (arm)
Cc: Alice Ryhl, Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module, linux-mm,
rust-for-linux, Christoph Hellwig
In-Reply-To: <507d24e0-4563-4b33-864c-cd1a499fe517@kernel.org>
On Thu, Feb 05, 2026 at 01:03:35PM +0100, David Hildenbrand (arm) wrote:
> On 2/5/26 12:57, Lorenzo Stoakes wrote:
> > +cc Christoph for his input on exports here.
> >
> > On Thu, Feb 05, 2026 at 12:43:03PM +0100, David Hildenbrand (arm) wrote:
> > > On 2/5/26 12:29, Lorenzo Stoakes wrote:
> > > >
> > > > Same point as before about exporting symbols, but given the _obj variants are
> > > > exported already this one is more valid.
> > > >
> > > >
> > > > Sorry but I don't want this exported at all.
> > > >
> > > > This is an internal implementation detail which allows fine-grained control of
> > > > behaviour via struct zap_details (which binder doesn't use, of course :)
> > >
> > > I don't expect anybody to set zap_details, but yeah, it could be abused.
> > > It could be abused right now from anywhere else in the kernel
> > > where we don't build as a module :)
> > >
> > > Apparently we export a similar function in rust where we just removed the last parameter.
> >
> > What??
> >
> > Alice - can you confirm rust isn't exporting stuff that isn't explicitly marked
> > EXPORT_SYMBOL*() for use by other rust modules?
> >
> > It's important we keep this in sync, otherwise rust is overriding kernel policy.
> >
> > >
> > > I think zap_page_range_single() is only called with non-NULL from mm/memory.c.
> > >
> > > So the following makes likely sense even outside of the context of this series:
> > >
> >
> > Yeah this looks good so feel free to add a R-b from me tag when you send it
> > BUT...
> >
> > I'm still _very_ uncomfortable with exporting this just for binder which seems
> > to be doing effectively mm tasks itself in a way that makes me think it needs a
> > rework to not be doing that and to update core mm to add functionality if it's
> > needed.
> >
> > In any case, if we _do_ export this I think I'm going to insist on this being
> > EXPORT_SYMBOL_FOR_MODULES() _only_ for the binder in-tree module.
>
> Works for me.
:)
>
> Staring at it again, I think I landed in cleanup land.
>
> zap_vma_ptes() is exported and does the same thing as
> zap_page_range_single(), just with some additional safety checks.
Yeah saw that, except it insists only on VM_PFN VMAs which makes me question our
making this more generally available to OOT drivers.
>
> Fun.
>
>
> Let me cleanup. Good finger exercise after one month of almost-not coding :)
:) I am less interested in cleanups at this stage at least for a while so feel
free to fixup glaringly horrible things so I can vicariously enjoy it at
least...
>
> --
> Cheers,
>
> David
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Lorenzo Stoakes @ 2026-02-05 12:10 UTC (permalink / raw)
To: Alice Ryhl
Cc: David Hildenbrand (arm), Greg Kroah-Hartman, Carlos Llamas,
Alexander Viro, Christian Brauner, Jan Kara, Paul Moore,
James Morris, Serge E. Hallyn, Andrew Morton, Dave Chinner,
Qi Zheng, Roman Gushchin, Muchun Song, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <aYSFyH-1kkW92M2N@google.com>
On Thu, Feb 05, 2026 at 11:58:00AM +0000, Alice Ryhl wrote:
> On Thu, Feb 05, 2026 at 12:43:03PM +0100, David Hildenbrand (arm) wrote:
> > On 2/5/26 12:29, Lorenzo Stoakes wrote:
> > > On Thu, Feb 05, 2026 at 10:51:28AM +0000, Alice Ryhl wrote:
> > > > bool list_lru_del_obj(struct list_lru *lru, struct list_head *item)
> > > > {
> > > > diff --git a/mm/memory.c b/mm/memory.c
> > > > index da360a6eb8a48e29293430d0c577fb4b6ec58099..64083ace239a2caf58e1645dd5d91a41d61492c4 100644
> > > > --- a/mm/memory.c
> > > > +++ b/mm/memory.c
> > > > @@ -2168,6 +2168,7 @@ void zap_page_range_single(struct vm_area_struct *vma, unsigned long address,
> > > > zap_page_range_single_batched(&tlb, vma, address, size, details);
> > > > tlb_finish_mmu(&tlb);
> > > > }
> > > > +EXPORT_SYMBOL(zap_page_range_single);
> > >
> > > Sorry but I don't want this exported at all.
> > >
> > > This is an internal implementation detail which allows fine-grained control of
> > > behaviour via struct zap_details (which binder doesn't use, of course :)
> >
> > I don't expect anybody to set zap_details, but yeah, it could be abused.
> > It could be abused right now from anywhere else in the kernel
> > where we don't build as a module :)
> >
> > Apparently we export a similar function in rust where we just removed the last parameter.
>
> To clarify, said Rust function gets inlined into Rust Binder, so Rust
> Binder calls the zap_page_range_single() symbol directly.
Presumably only for things compiled into the kernel right?
^ permalink raw reply
* Re: [PATCH 3/5] mm: export zap_page_range_single and list_lru_add/del
From: Lorenzo Stoakes @ 2026-02-05 12:07 UTC (permalink / raw)
To: David Hildenbrand (arm)
Cc: Alice Ryhl, Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Dave Chinner, Qi Zheng,
Roman Gushchin, Muchun Song, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module, linux-mm,
rust-for-linux
In-Reply-To: <d309b3f6-8959-4767-b262-7fdb957669aa@kernel.org>
On Thu, Feb 05, 2026 at 01:06:10PM +0100, David Hildenbrand (arm) wrote:
> On 2/5/26 13:01, Lorenzo Stoakes wrote:
> > On Thu, Feb 05, 2026 at 12:57:04PM +0100, David Hildenbrand (arm) wrote:
> >
> > Dude you have to capitalise that 'a' in Arm it's driving me crazy ;)
> >
> > Then again should it be ARM? OK this is tricky
>
> Yeah, I should likely adjust that at some point. For the time being, I enjoy
> driving you crazy :P
:D
>
> >
> > [snip]
> >
> > >
> > > The following should compile :)
> >
> > Err... yeah. OK my R-b obviously depends on the code being compiling + working
> > :P But still feel free to add when you break it out + _test_ it ;)
>
> Better to add your R-b when I send it out officially :)
Haha yeah maybe...
>
> --
> Cheers,
>
> David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox