[PATCH 2/4] spufs: fix gang directory lifetimes

2025-03-13 Thread Al Viro
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 
---
 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,

Re: [PATCH 2/4] spufs: fix gang directory lifetimes

2025-03-13 Thread Christian Brauner
On Thu, Mar 13, 2025 at 04:29:01AM +, 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 
> ---

Reviewed-by: Christian Brauner