* [PATCHES] several fixes from tree-in-dcache stuff
@ 2025-03-13 4:27 Al Viro
2025-03-13 4:28 ` [PATCH 1/4] spufs: fix a leak on spufs_new_file() failure Al Viro
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Al Viro @ 2025-03-13 4:27 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linuxppc-dev, linux-rdma
Several fixes for fairly old crap - qibfs leak, a couple
of spufs ones and a spufs double-dput() memory corruptor.
This stuff sits in viro/vfs.git#fixes; individual patches
in followups. Review would be very welcome.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/4] spufs: fix a leak on spufs_new_file() failure
2025-03-13 4:27 [PATCHES] several fixes from tree-in-dcache stuff Al Viro
@ 2025-03-13 4:28 ` Al Viro
2025-03-13 8:26 ` Christian Brauner
2025-03-13 4:29 ` [PATCH 2/4] spufs: fix gang directory lifetimes Al Viro
2025-03-13 4:29 ` [PATCH 3/4] spufs: fix a leak in spufs_create_context() Al Viro
2 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2025-03-13 4:28 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linuxppc-dev
It's called from spufs_fill_dir(), and caller of that will do
spufs_rmdir() in case of failure. That does remove everything
we'd managed to create, but... the problem dentry is still
negative. IOW, it needs to be explicitly dropped.
Fixes: 3f51dd91c807 "[PATCH] spufs: fix spufs_fill_dir error path"
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
arch/powerpc/platforms/cell/spufs/inode.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index 70236d1df3d3..793c005607cf 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -192,8 +192,10 @@ static int spufs_fill_dir(struct dentry *dir,
return -ENOMEM;
ret = spufs_new_file(dir->d_sb, dentry, files->ops,
files->mode & mode, files->size, ctx);
- if (ret)
+ if (ret) {
+ dput(dentry);
return ret;
+ }
files++;
}
return 0;
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] spufs: fix gang directory lifetimes
2025-03-13 4:27 [PATCHES] several fixes from tree-in-dcache stuff Al Viro
2025-03-13 4:28 ` [PATCH 1/4] spufs: fix a leak on spufs_new_file() failure Al Viro
@ 2025-03-13 4:29 ` Al Viro
2025-03-13 8:28 ` Christian Brauner
2025-03-13 4:29 ` [PATCH 3/4] spufs: fix a leak in spufs_create_context() Al Viro
2 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2025-03-13 4:29 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linuxppc-dev
prior to "[POWERPC] spufs: Fix gang destroy leaks" we used to have
a problem with gang lifetimes - creation of a gang returns opened
gang directory, which normally gets removed when that gets closed,
but if somebody has created a context belonging to that gang and
kept it alive until the gang got closed, removal failed and we
ended up with a leak.
Unfortunately, it had been fixed the wrong way. Dentry of gang
directory was no longer pinned, and rmdir on close was gone.
One problem was that failure of open kept calling simple_rmdir()
as cleanup, which meant an unbalanced dput(). Another bug was
in the success case - gang creation incremented link count on
root directory, but that was no longer undone when gang got
destroyed.
Fix consists of
* reverting the commit in question
* adding a counter to gang, protected by ->i_rwsem
of gang directory inode.
* having it set to 1 at creation time, dropped
in both spufs_dir_close() and spufs_gang_close() and bumped
in spufs_create_context(), provided that it's not 0.
* using simple_recursive_removal() to take the gang
directory out when counter reaches zero.
Fixes: 877907d37da9 "[POWERPC] spufs: Fix gang destroy leaks"
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
arch/powerpc/platforms/cell/spufs/gang.c | 1 +
arch/powerpc/platforms/cell/spufs/inode.c | 54 +++++++++++++++++++----
arch/powerpc/platforms/cell/spufs/spufs.h | 2 +
3 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/gang.c b/arch/powerpc/platforms/cell/spufs/gang.c
index 827d338deaf4..2c2999de6bfa 100644
--- a/arch/powerpc/platforms/cell/spufs/gang.c
+++ b/arch/powerpc/platforms/cell/spufs/gang.c
@@ -25,6 +25,7 @@ struct spu_gang *alloc_spu_gang(void)
mutex_init(&gang->aff_mutex);
INIT_LIST_HEAD(&gang->list);
INIT_LIST_HEAD(&gang->aff_list_head);
+ gang->alive = 1;
out:
return gang;
diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index 793c005607cf..c566e7997f2c 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -201,6 +201,23 @@ static int spufs_fill_dir(struct dentry *dir,
return 0;
}
+static void unuse_gang(struct dentry *dir)
+{
+ struct inode *inode = dir->d_inode;
+ struct spu_gang *gang = SPUFS_I(inode)->i_gang;
+
+ if (gang) {
+ bool dead;
+
+ inode_lock(inode); // exclusion with spufs_create_context()
+ dead = !--gang->alive;
+ inode_unlock(inode);
+
+ if (dead)
+ simple_recursive_removal(dir, NULL);
+ }
+}
+
static int spufs_dir_close(struct inode *inode, struct file *file)
{
struct inode *parent;
@@ -215,6 +232,7 @@ static int spufs_dir_close(struct inode *inode, struct file *file)
inode_unlock(parent);
WARN_ON(ret);
+ unuse_gang(dir->d_parent);
return dcache_dir_close(inode, file);
}
@@ -407,7 +425,7 @@ spufs_create_context(struct inode *inode, struct dentry *dentry,
{
int ret;
int affinity;
- struct spu_gang *gang;
+ struct spu_gang *gang = SPUFS_I(inode)->i_gang;
struct spu_context *neighbor;
struct path path = {.mnt = mnt, .dentry = dentry};
@@ -422,11 +440,15 @@ spufs_create_context(struct inode *inode, struct dentry *dentry,
if ((flags & SPU_CREATE_ISOLATE) && !isolated_loader)
return -ENODEV;
- gang = NULL;
+ if (gang) {
+ if (!gang->alive)
+ return -ENOENT;
+ gang->alive++;
+ }
+
neighbor = NULL;
affinity = flags & (SPU_CREATE_AFFINITY_MEM | SPU_CREATE_AFFINITY_SPU);
if (affinity) {
- gang = SPUFS_I(inode)->i_gang;
if (!gang)
return -EINVAL;
mutex_lock(&gang->aff_mutex);
@@ -455,6 +477,8 @@ spufs_create_context(struct inode *inode, struct dentry *dentry,
out_aff_unlock:
if (affinity)
mutex_unlock(&gang->aff_mutex);
+ if (ret && gang)
+ gang->alive--; // can't reach 0
return ret;
}
@@ -484,6 +508,7 @@ spufs_mkgang(struct inode *dir, struct dentry *dentry, umode_t mode)
inode->i_fop = &simple_dir_operations;
d_instantiate(dentry, inode);
+ dget(dentry);
inc_nlink(dir);
inc_nlink(d_inode(dentry));
return ret;
@@ -494,6 +519,21 @@ spufs_mkgang(struct inode *dir, struct dentry *dentry, umode_t mode)
return ret;
}
+static int spufs_gang_close(struct inode *inode, struct file *file)
+{
+ unuse_gang(file->f_path.dentry);
+ return dcache_dir_close(inode, file);
+}
+
+static const struct file_operations spufs_gang_fops = {
+ .open = dcache_dir_open,
+ .release = spufs_gang_close,
+ .llseek = dcache_dir_lseek,
+ .read = generic_read_dir,
+ .iterate_shared = dcache_readdir,
+ .fsync = noop_fsync,
+};
+
static int spufs_gang_open(const struct path *path)
{
int ret;
@@ -513,7 +553,7 @@ static int spufs_gang_open(const struct path *path)
return PTR_ERR(filp);
}
- filp->f_op = &simple_dir_operations;
+ filp->f_op = &spufs_gang_fops;
fd_install(ret, filp);
return ret;
}
@@ -528,10 +568,8 @@ static int spufs_create_gang(struct inode *inode,
ret = spufs_mkgang(inode, dentry, mode & 0777);
if (!ret) {
ret = spufs_gang_open(&path);
- if (ret < 0) {
- int err = simple_rmdir(inode, dentry);
- WARN_ON(err);
- }
+ if (ret < 0)
+ unuse_gang(dentry);
}
return ret;
}
diff --git a/arch/powerpc/platforms/cell/spufs/spufs.h b/arch/powerpc/platforms/cell/spufs/spufs.h
index 84958487f696..d33787c57c39 100644
--- a/arch/powerpc/platforms/cell/spufs/spufs.h
+++ b/arch/powerpc/platforms/cell/spufs/spufs.h
@@ -151,6 +151,8 @@ struct spu_gang {
int aff_flags;
struct spu *aff_ref_spu;
atomic_t aff_sched_count;
+
+ int alive;
};
/* Flag bits for spu_gang aff_flags */
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] spufs: fix a leak in spufs_create_context()
2025-03-13 4:27 [PATCHES] several fixes from tree-in-dcache stuff Al Viro
2025-03-13 4:28 ` [PATCH 1/4] spufs: fix a leak on spufs_new_file() failure Al Viro
2025-03-13 4:29 ` [PATCH 2/4] spufs: fix gang directory lifetimes Al Viro
@ 2025-03-13 4:29 ` Al Viro
2025-03-13 8:28 ` Christian Brauner
2 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2025-03-13 4:29 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linuxppc-dev
Leak fixes back in 2008 missed one case - if we are trying to set affinity
and spufs_mkdir() fails, we need to drop the reference to neighbor.
Fixes: 58119068cb27 "[POWERPC] spufs: Fix memory leak on SPU affinity"
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
arch/powerpc/platforms/cell/spufs/inode.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index c566e7997f2c..9f9e4b871627 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -460,8 +460,11 @@ spufs_create_context(struct inode *inode, struct dentry *dentry,
}
ret = spufs_mkdir(inode, dentry, flags, mode & 0777);
- if (ret)
+ if (ret) {
+ if (neighbor)
+ put_spu_context(neighbor);
goto out_aff_unlock;
+ }
if (affinity) {
spufs_set_affinity(flags, SPUFS_I(d_inode(dentry))->i_ctx,
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] spufs: fix a leak on spufs_new_file() failure
2025-03-13 4:28 ` [PATCH 1/4] spufs: fix a leak on spufs_new_file() failure Al Viro
@ 2025-03-13 8:26 ` Christian Brauner
0 siblings, 0 replies; 7+ messages in thread
From: Christian Brauner @ 2025-03-13 8:26 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linuxppc-dev
On Thu, Mar 13, 2025 at 04:28:15AM +0000, Al Viro wrote:
> It's called from spufs_fill_dir(), and caller of that will do
> spufs_rmdir() in case of failure. That does remove everything
> we'd managed to create, but... the problem dentry is still
> negative. IOW, it needs to be explicitly dropped.
>
> Fixes: 3f51dd91c807 "[PATCH] spufs: fix spufs_fill_dir error path"
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] spufs: fix gang directory lifetimes
2025-03-13 4:29 ` [PATCH 2/4] spufs: fix gang directory lifetimes Al Viro
@ 2025-03-13 8:28 ` Christian Brauner
0 siblings, 0 replies; 7+ messages in thread
From: Christian Brauner @ 2025-03-13 8:28 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linuxppc-dev
On Thu, Mar 13, 2025 at 04:29:01AM +0000, Al Viro wrote:
> prior to "[POWERPC] spufs: Fix gang destroy leaks" we used to have
> a problem with gang lifetimes - creation of a gang returns opened
> gang directory, which normally gets removed when that gets closed,
> but if somebody has created a context belonging to that gang and
> kept it alive until the gang got closed, removal failed and we
> ended up with a leak.
>
> Unfortunately, it had been fixed the wrong way. Dentry of gang
> directory was no longer pinned, and rmdir on close was gone.
> One problem was that failure of open kept calling simple_rmdir()
> as cleanup, which meant an unbalanced dput(). Another bug was
> in the success case - gang creation incremented link count on
> root directory, but that was no longer undone when gang got
> destroyed.
>
> Fix consists of
> * reverting the commit in question
> * adding a counter to gang, protected by ->i_rwsem
> of gang directory inode.
> * having it set to 1 at creation time, dropped
> in both spufs_dir_close() and spufs_gang_close() and bumped
> in spufs_create_context(), provided that it's not 0.
> * using simple_recursive_removal() to take the gang
> directory out when counter reaches zero.
>
> Fixes: 877907d37da9 "[POWERPC] spufs: Fix gang destroy leaks"
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] spufs: fix a leak in spufs_create_context()
2025-03-13 4:29 ` [PATCH 3/4] spufs: fix a leak in spufs_create_context() Al Viro
@ 2025-03-13 8:28 ` Christian Brauner
0 siblings, 0 replies; 7+ messages in thread
From: Christian Brauner @ 2025-03-13 8:28 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linuxppc-dev
On Thu, Mar 13, 2025 at 04:29:32AM +0000, Al Viro wrote:
> Leak fixes back in 2008 missed one case - if we are trying to set affinity
> and spufs_mkdir() fails, we need to drop the reference to neighbor.
>
> Fixes: 58119068cb27 "[POWERPC] spufs: Fix memory leak on SPU affinity"
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
Reviewed-by: Christian Brauner <brauner@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-03-13 8:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-13 4:27 [PATCHES] several fixes from tree-in-dcache stuff Al Viro
2025-03-13 4:28 ` [PATCH 1/4] spufs: fix a leak on spufs_new_file() failure Al Viro
2025-03-13 8:26 ` Christian Brauner
2025-03-13 4:29 ` [PATCH 2/4] spufs: fix gang directory lifetimes Al Viro
2025-03-13 8:28 ` Christian Brauner
2025-03-13 4:29 ` [PATCH 3/4] spufs: fix a leak in spufs_create_context() Al Viro
2025-03-13 8:28 ` Christian Brauner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).