To simplify crash recovery and reduce performance impact, backing_ids
are not persisted across daemon restarts. After crash recovery, this
may lead to resource leaks if backing file resources are not properly
cleaned up.

Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
and put backing files. When the FUSE daemon restarts, it can use this
ioctl to cleanup all backing file resources.

Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
---
 fs/fuse/backing.c         | 19 +++++++++++++++++++
 fs/fuse/dev.c             | 16 ++++++++++++++++
 fs/fuse/fuse_i.h          |  1 +
 include/uapi/linux/fuse.h |  1 +
 4 files changed, 37 insertions(+)

diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
index 4afda419dd14..e93d797a2cde 100644
--- a/fs/fuse/backing.c
+++ b/fs/fuse/backing.c
@@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
 	return err;
 }
 
+static int fuse_backing_close_one(int id, void *p, void *data)
+{
+	struct fuse_conn *fc = data;
+
+	fuse_backing_close(fc, id);
+
+	return 0;
+}
+
+int fuse_backing_close_all(struct fuse_conn *fc)
+{
+	if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
+
+	return 0;
+}
+
 struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc, int backing_id)
 {
 	struct fuse_backing *fb;
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 6d59cbc877c6..f05d55302598 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2654,6 +2654,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
 	return fuse_backing_close(fud->fc, backing_id);
 }
 
+static long fuse_dev_ioctl_backing_close_all(struct file *file)
+{
+	struct fuse_dev *fud = fuse_get_dev(file);
+
+	if (IS_ERR(fud))
+		return PTR_ERR(fud);
+
+	if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
+		return -EOPNOTSUPP;
+
+	return fuse_backing_close_all(fud->fc);
+}
+
 static long fuse_dev_ioctl_sync_init(struct file *file)
 {
 	int err = -EINVAL;
@@ -2682,6 +2695,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
 	case FUSE_DEV_IOC_BACKING_CLOSE:
 		return fuse_dev_ioctl_backing_close(file, argp);
 
+	case FUSE_DEV_IOC_BACKING_CLOSE_ALL:
+		return fuse_dev_ioctl_backing_close_all(file);
+
 	case FUSE_DEV_IOC_SYNC_INIT:
 		return fuse_dev_ioctl_sync_init(file);
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7f16049387d1..33e91a5d3765 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1573,6 +1573,7 @@ void fuse_backing_files_init(struct fuse_conn *fc);
 void fuse_backing_files_free(struct fuse_conn *fc);
 int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map);
 int fuse_backing_close(struct fuse_conn *fc, int backing_id);
+int fuse_backing_close_all(struct fuse_conn *fc);
 
 /* passthrough.c */
 static inline struct fuse_backing *fuse_inode_backing(struct fuse_inode *fi)
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index c13e1f9a2f12..e4ff28a4ff40 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -1139,6 +1139,7 @@ struct fuse_backing_map {
 					     struct fuse_backing_map)
 #define FUSE_DEV_IOC_BACKING_CLOSE	_IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t)
 #define FUSE_DEV_IOC_SYNC_INIT		_IO(FUSE_DEV_IOC_MAGIC, 3)
+#define FUSE_DEV_IOC_BACKING_CLOSE_ALL	_IO(FUSE_DEV_IOC_MAGIC, 4)
 
 struct fuse_lseek_in {
 	uint64_t	fh;
-- 
2.43.0

On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>
> To simplify crash recovery and reduce performance impact, backing_ids
> are not persisted across daemon restarts. After crash recovery, this
> may lead to resource leaks if backing file resources are not properly
> cleaned up.
>
> Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
> and put backing files. When the FUSE daemon restarts, it can use this
> ioctl to cleanup all backing file resources.
>
> Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
> ---
>  fs/fuse/backing.c         | 19 +++++++++++++++++++
>  fs/fuse/dev.c             | 16 ++++++++++++++++
>  fs/fuse/fuse_i.h          |  1 +
>  include/uapi/linux/fuse.h |  1 +
>  4 files changed, 37 insertions(+)
>
> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> index 4afda419dd14..e93d797a2cde 100644
> --- a/fs/fuse/backing.c
> +++ b/fs/fuse/backing.c
> @@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
>         return err;
>  }
>
> +static int fuse_backing_close_one(int id, void *p, void *data)
> +{
> +       struct fuse_conn *fc = data;
> +
> +       fuse_backing_close(fc, id);
> +
> +       return 0;
> +}
> +
> +int fuse_backing_close_all(struct fuse_conn *fc)
> +{
> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> +               return -EPERM;
> +
> +       idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
> +
> +       return 0;
> +}
> +

This is not safe and not efficient.
For safety from racing with _open/_close, iteration needs at least
rcu_read_lock(),
but I think it will be much more efficient to zap the entire map with
fuse_backing_files_free()/fuse_backing_files_init().

This of course needs to be synchronized with concurrent _open/_close/_lookup.
This could be done by making c->backing_files_map a struct idr __rcu *
and replace the old and new backing_files_map under spin_lock(&fc->lock);

Then you can call fuse_backing_files_free() on the old backing_files_map
without a lock.

As a side note, fuse_backing_files_free() iteration looks like it may need
cond_resched() if there are a LOT of backing ids, but I am not sure and
this is orthogonal to your change.

Thanks,
Amir.

On 1/16/26 11:39 PM, Amir Goldstein wrote:
> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>
>> To simplify crash recovery and reduce performance impact, backing_ids
>> are not persisted across daemon restarts. After crash recovery, this
>> may lead to resource leaks if backing file resources are not properly
>> cleaned up.
>>
>> Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
>> and put backing files. When the FUSE daemon restarts, it can use this
>> ioctl to cleanup all backing file resources.
>>
>> Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
>> ---
>>   fs/fuse/backing.c         | 19 +++++++++++++++++++
>>   fs/fuse/dev.c             | 16 ++++++++++++++++
>>   fs/fuse/fuse_i.h          |  1 +
>>   include/uapi/linux/fuse.h |  1 +
>>   4 files changed, 37 insertions(+)
>>
>> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
>> index 4afda419dd14..e93d797a2cde 100644
>> --- a/fs/fuse/backing.c
>> +++ b/fs/fuse/backing.c
>> @@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
>>          return err;
>>   }
>>
>> +static int fuse_backing_close_one(int id, void *p, void *data)
>> +{
>> +       struct fuse_conn *fc = data;
>> +
>> +       fuse_backing_close(fc, id);
>> +
>> +       return 0;
>> +}
>> +
>> +int fuse_backing_close_all(struct fuse_conn *fc)
>> +{
>> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
>> +               return -EPERM;
>> +
>> +       idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
>> +
>> +       return 0;
>> +}
>> +
> 
> This is not safe and not efficient.
> For safety from racing with _open/_close, iteration needs at least
> rcu_read_lock(),

Yes, you're absolutely right. Additionally, calling idr_remove within 
idr_for_each maybe presents safety risks.

> but I think it will be much more efficient to zap the entire map with
> fuse_backing_files_free()/fuse_backing_files_init().
> 
> This of course needs to be synchronized with concurrent _open/_close/_lookup.
> This could be done by making c->backing_files_map a struct idr __rcu *
> and replace the old and new backing_files_map under spin_lock(&fc->lock);
> 
> Then you can call fuse_backing_files_free() on the old backing_files_map
> without a lock.
> 
> As a side note, fuse_backing_files_free() iteration looks like it may need
> cond_resched() if there are a LOT of backing ids, but I am not sure and
> this is orthogonal to your change.
> 
> Thanks,
> Amir.
> 
> 

Thank you for your helpful suggestions. However, it cannot use 
fuse_backing_files_free() in the close_all implementation because it 
directly frees backing files without respecting reference counts. This 
function requires that no one is actively using the backing file (it 
even has WARN_ON_ONCE(refcount_read(&fb->count) != 1)), which cannot be 
guaranteed after a crash recovery scenario where backing files may still 
be in use.

Instead, the implementation uses fuse_backing_put() to safely decrement 
the reference count and allow the backing file to be freed when no 
longer in use.

Additionally, the implementation addresses two race conditions:

- Race between idr_for_each and lookup: Uses synchronize_rcu() to ensure 
all concurrent RCU readers (i.e., in-flight fuse_backing_lookup() calls) 
complete before releasing backing files, preventing use-after-free issues.

- Race with open/close operations: Uses fc->lock to atomically swap the 
old and new IDR maps, ensuring consistency with concurrent 
fuse_backing_open() and fuse_backing_close() operations.

This approach provides the same as the RCU pointer suggestion, but with 
less code and no changes to the struct fuse_conn data structures.

I've updated it and verified the implementation. Could you please review it?


diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
index 4afda419dd14..047d373684f9 100644
--- a/fs/fuse/backing.c
+++ b/fs/fuse/backing.c
@@ -166,6 +166,45 @@ int fuse_backing_close(struct fuse_conn *fc, int 
backing_id)
         return err;
  }

+static int fuse_backing_release_one(int id, void *p, void *data)
+{
+       struct fuse_backing *fb = p;
+
+       fuse_backing_put(fb);
+
+       return 0;
+}
+
+int fuse_backing_close_all(struct fuse_conn *fc)
+{
+       struct idr old_map;
+
+       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
+               return -EPERM;
+
+       /*
+        * Swap out the old backing_files_map with a new empty one under 
lock,
+        * then release all backing files outside the lock. This avoids long
+        * lock hold times and potential races with concurrent open/close
+        * operations.
+        */
+       idr_init(&old_map);
+       spin_lock(&fc->lock);
+       swap(fc->backing_files_map, old_map);
+       spin_unlock(&fc->lock);
+
+       /*
+        * Ensure all concurrent RCU readers complete before releasing 
backing
+        * files, so any in-flight lookups can safely take references.
+        */
+       synchronize_rcu();
+
+       idr_for_each(&old_map, fuse_backing_release_one, NULL);
+       idr_destroy(&old_map);
+
+       return 0;
+}
+

--

Thanks,
Chunsheng Luo

On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>
>
>
> On 1/16/26 11:39 PM, Amir Goldstein wrote:
> > On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> >>
> >> To simplify crash recovery and reduce performance impact, backing_ids
> >> are not persisted across daemon restarts. After crash recovery, this
> >> may lead to resource leaks if backing file resources are not properly
> >> cleaned up.
> >>
> >> Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
> >> and put backing files. When the FUSE daemon restarts, it can use this
> >> ioctl to cleanup all backing file resources.
> >>
> >> Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
> >> ---
> >>   fs/fuse/backing.c         | 19 +++++++++++++++++++
> >>   fs/fuse/dev.c             | 16 ++++++++++++++++
> >>   fs/fuse/fuse_i.h          |  1 +
> >>   include/uapi/linux/fuse.h |  1 +
> >>   4 files changed, 37 insertions(+)
> >>
> >> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> >> index 4afda419dd14..e93d797a2cde 100644
> >> --- a/fs/fuse/backing.c
> >> +++ b/fs/fuse/backing.c
> >> @@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
> >>          return err;
> >>   }
> >>
> >> +static int fuse_backing_close_one(int id, void *p, void *data)
> >> +{
> >> +       struct fuse_conn *fc = data;
> >> +
> >> +       fuse_backing_close(fc, id);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +int fuse_backing_close_all(struct fuse_conn *fc)
> >> +{
> >> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> >> +               return -EPERM;
> >> +
> >> +       idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >
> > This is not safe and not efficient.
> > For safety from racing with _open/_close, iteration needs at least
> > rcu_read_lock(),
>
> Yes, you're absolutely right. Additionally, calling idr_remove within
> idr_for_each maybe presents safety risks.
>
> > but I think it will be much more efficient to zap the entire map with
> > fuse_backing_files_free()/fuse_backing_files_init().
> >
> > This of course needs to be synchronized with concurrent _open/_close/_lookup.
> > This could be done by making c->backing_files_map a struct idr __rcu *
> > and replace the old and new backing_files_map under spin_lock(&fc->lock);
> >
> > Then you can call fuse_backing_files_free() on the old backing_files_map
> > without a lock.
> >
> > As a side note, fuse_backing_files_free() iteration looks like it may need
> > cond_resched() if there are a LOT of backing ids, but I am not sure and
> > this is orthogonal to your change.
> >
> > Thanks,
> > Amir.
> >
> >
>
> Thank you for your helpful suggestions. However, it cannot use
> fuse_backing_files_free() in the close_all implementation because it
> directly frees backing files without respecting reference counts. This
> function requires that no one is actively using the backing file (it
> even has WARN_ON_ONCE(refcount_read(&fb->count) != 1)), which cannot be
> guaranteed after a crash recovery scenario where backing files may still
> be in use.

Right.

>
> Instead, the implementation uses fuse_backing_put() to safely decrement
> the reference count and allow the backing file to be freed when no
> longer in use.

OK.

>
> Additionally, the implementation addresses two race conditions:
>
> - Race between idr_for_each and lookup: Uses synchronize_rcu() to ensure
> all concurrent RCU readers (i.e., in-flight fuse_backing_lookup() calls)
> complete before releasing backing files, preventing use-after-free issues.

Almost. See below.

>
> - Race with open/close operations: Uses fc->lock to atomically swap the
> old and new IDR maps, ensuring consistency with concurrent
> fuse_backing_open() and fuse_backing_close() operations.
>
> This approach provides the same as the RCU pointer suggestion, but with
> less code and no changes to the struct fuse_conn data structures.
>
> I've updated it and verified the implementation. Could you please review it?
>
>
> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> index 4afda419dd14..047d373684f9 100644
> --- a/fs/fuse/backing.c
> +++ b/fs/fuse/backing.c
> @@ -166,6 +166,45 @@ int fuse_backing_close(struct fuse_conn *fc, int
> backing_id)
>          return err;
>   }
>
> +static int fuse_backing_release_one(int id, void *p, void *data)
> +{
> +       struct fuse_backing *fb = p;
> +
> +       fuse_backing_put(fb);
> +
> +       return 0;
> +}
> +
> +int fuse_backing_close_all(struct fuse_conn *fc)
> +{
> +       struct idr old_map;
> +
> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> +               return -EPERM;
> +
> +       /*
> +        * Swap out the old backing_files_map with a new empty one under
> lock,
> +        * then release all backing files outside the lock. This avoids long
> +        * lock hold times and potential races with concurrent open/close
> +        * operations.
> +        */
> +       idr_init(&old_map);
> +       spin_lock(&fc->lock);
> +       swap(fc->backing_files_map, old_map);
> +       spin_unlock(&fc->lock);
> +
> +       /*
> +        * Ensure all concurrent RCU readers complete before releasing
> backing
> +        * files, so any in-flight lookups can safely take references.
> +        */
> +       synchronize_rcu();
> +
> +       idr_for_each(&old_map, fuse_backing_release_one, NULL);
> +       idr_destroy(&old_map);
> +
> +       return 0;
> +}
> +

That's almost safe but not enough.
This lookup code is not safe against the swap():

  rcu_read_lock();
  fb = idr_find(&fc->backing_files_map, backing_id);

That is the reason you need to make fc->backing_files_map
an rcu referenced ptr.

Instead of swap() you use xchg() to atomically exchange the
old and new struct idr pointers and for lookup:

  rcu_read_lock();
  fb = idr_find(rcu_dereference(fc->backing_files_map), backing_id);

Thanks,
Amir.

On 1/18/26 1:00 AM, Amir Goldstein wrote:
> On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>
>>
>>
>> On 1/16/26 11:39 PM, Amir Goldstein wrote:
>>> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>>>
>>>> To simplify crash recovery and reduce performance impact, backing_ids
>>>> are not persisted across daemon restarts. After crash recovery, this
>>>> may lead to resource leaks if backing file resources are not properly
>>>> cleaned up.
>>>>
>>>> Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
>>>> and put backing files. When the FUSE daemon restarts, it can use this
>>>> ioctl to cleanup all backing file resources.
>>>>
>>>> Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
>>>> ---
>>>>    fs/fuse/backing.c         | 19 +++++++++++++++++++
>>>>    fs/fuse/dev.c             | 16 ++++++++++++++++
>>>>    fs/fuse/fuse_i.h          |  1 +
>>>>    include/uapi/linux/fuse.h |  1 +
>>>>    4 files changed, 37 insertions(+)
>>>>
>>>> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
>>>> index 4afda419dd14..e93d797a2cde 100644
>>>> --- a/fs/fuse/backing.c
>>>> +++ b/fs/fuse/backing.c
>>>> @@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
>>>>           return err;
>>>>    }
>>>>
>>>> +static int fuse_backing_close_one(int id, void *p, void *data)
>>>> +{
>>>> +       struct fuse_conn *fc = data;
>>>> +
>>>> +       fuse_backing_close(fc, id);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +int fuse_backing_close_all(struct fuse_conn *fc)
>>>> +{
>>>> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
>>>> +               return -EPERM;
>>>> +
>>>> +       idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>
>>> This is not safe and not efficient.
>>> For safety from racing with _open/_close, iteration needs at least
>>> rcu_read_lock(),
>>
>> Yes, you're absolutely right. Additionally, calling idr_remove within
>> idr_for_each maybe presents safety risks.
>>
>>> but I think it will be much more efficient to zap the entire map with
>>> fuse_backing_files_free()/fuse_backing_files_init().
>>>
>>> This of course needs to be synchronized with concurrent _open/_close/_lookup.
>>> This could be done by making c->backing_files_map a struct idr __rcu *
>>> and replace the old and new backing_files_map under spin_lock(&fc->lock);
>>>
>>> Then you can call fuse_backing_files_free() on the old backing_files_map
>>> without a lock.
>>>
>>> As a side note, fuse_backing_files_free() iteration looks like it may need
>>> cond_resched() if there are a LOT of backing ids, but I am not sure and
>>> this is orthogonal to your change.
>>>
>>> Thanks,
>>> Amir.
>>>
>>>
>>
>> Thank you for your helpful suggestions. However, it cannot use
>> fuse_backing_files_free() in the close_all implementation because it
>> directly frees backing files without respecting reference counts. This
>> function requires that no one is actively using the backing file (it
>> even has WARN_ON_ONCE(refcount_read(&fb->count) != 1)), which cannot be
>> guaranteed after a crash recovery scenario where backing files may still
>> be in use.
> 
> Right.
> 
>>
>> Instead, the implementation uses fuse_backing_put() to safely decrement
>> the reference count and allow the backing file to be freed when no
>> longer in use.
> 
> OK.
> 
>>
>> Additionally, the implementation addresses two race conditions:
>>
>> - Race between idr_for_each and lookup: Uses synchronize_rcu() to ensure
>> all concurrent RCU readers (i.e., in-flight fuse_backing_lookup() calls)
>> complete before releasing backing files, preventing use-after-free issues.
> 
> Almost. See below.
> 
>>
>> - Race with open/close operations: Uses fc->lock to atomically swap the
>> old and new IDR maps, ensuring consistency with concurrent
>> fuse_backing_open() and fuse_backing_close() operations.
>>
>> This approach provides the same as the RCU pointer suggestion, but with
>> less code and no changes to the struct fuse_conn data structures.
>>
>> I've updated it and verified the implementation. Could you please review it?
>>
>>
>> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
>> index 4afda419dd14..047d373684f9 100644
>> --- a/fs/fuse/backing.c
>> +++ b/fs/fuse/backing.c
>> @@ -166,6 +166,45 @@ int fuse_backing_close(struct fuse_conn *fc, int
>> backing_id)
>>           return err;
>>    }
>>
>> +static int fuse_backing_release_one(int id, void *p, void *data)
>> +{
>> +       struct fuse_backing *fb = p;
>> +
>> +       fuse_backing_put(fb);
>> +
>> +       return 0;
>> +}
>> +
>> +int fuse_backing_close_all(struct fuse_conn *fc)
>> +{
>> +       struct idr old_map;
>> +
>> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
>> +               return -EPERM;
>> +
>> +       /*
>> +        * Swap out the old backing_files_map with a new empty one under
>> lock,
>> +        * then release all backing files outside the lock. This avoids long
>> +        * lock hold times and potential races with concurrent open/close
>> +        * operations.
>> +        */
>> +       idr_init(&old_map);
>> +       spin_lock(&fc->lock);
>> +       swap(fc->backing_files_map, old_map);
>> +       spin_unlock(&fc->lock);
>> +
>> +       /*
>> +        * Ensure all concurrent RCU readers complete before releasing
>> backing
>> +        * files, so any in-flight lookups can safely take references.
>> +        */
>> +       synchronize_rcu();
>> +
>> +       idr_for_each(&old_map, fuse_backing_release_one, NULL);
>> +       idr_destroy(&old_map);
>> +
>> +       return 0;
>> +}
>> +
> 
> That's almost safe but not enough.
> This lookup code is not safe against the swap():
> 
>    rcu_read_lock();
>    fb = idr_find(&fc->backing_files_map, backing_id);
> 
> That is the reason you need to make fc->backing_files_map
> an rcu referenced ptr.
> 
> Instead of swap() you use xchg() to atomically exchange the
> old and new struct idr pointers and for lookup:
> 
>    rcu_read_lock();
>    fb = idr_find(rcu_dereference(fc->backing_files_map), backing_id);
> 
> Thanks,
> Amir.
> 
> 

Yes, swap() isn't atomic, it's just copying structs, so it's not safe 
when racing with lookup.

I've updated the version to make fc->backing_files_map an rcu referenced 
ptr. Please review the attached patch.

Thanks,
Chunsheng Luo.From b674dcef4e58318c92b061c62899d5598203547e Mon Sep 17 00:00:00 2001
From: Chunsheng Luo <luochunsheng@ustc.edu>
Date: Mon, 12 Jan 2026 16:56:36 +0800
Subject: [PATCH] fuse: add ioctl to cleanup all backing files

To simplify crash recovery and reduce performance impact, backing_ids
are not persisted across daemon restarts. After crash recovery, this
may lead to resource leaks if backing file resources are not properly
cleaned up.

Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
and put backing files. When the FUSE daemon restarts, it can use this
ioctl to cleanup all backing file resources.

Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
---
 fs/fuse/backing.c         | 77 +++++++++++++++++++++++++++++++++++----
 fs/fuse/dev.c             | 16 ++++++++
 fs/fuse/fuse_i.h          |  5 ++-
 fs/fuse/inode.c           | 11 +++---
 include/uapi/linux/fuse.h |  1 +
 5 files changed, 95 insertions(+), 15 deletions(-)

diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
index 4afda419dd14..2da024dc003d 100644
--- a/fs/fuse/backing.c
+++ b/fs/fuse/backing.c
@@ -32,19 +32,29 @@ void fuse_backing_put(struct fuse_backing *fb)
 		fuse_backing_free(fb);
 }
 
-void fuse_backing_files_init(struct fuse_conn *fc)
+int fuse_backing_files_init(struct fuse_conn *fc)
 {
-	idr_init(&fc->backing_files_map);
+	struct idr *idr;
+
+	idr = kzalloc(sizeof(*idr), GFP_KERNEL);
+	if (!idr)
+		return -ENOMEM;
+	idr_init(idr);
+	rcu_assign_pointer(fc->backing_files_map, idr);
+	return 0;
 }
 
 static int fuse_backing_id_alloc(struct fuse_conn *fc, struct fuse_backing *fb)
 {
+	struct idr *idr;
 	int id;
 
 	idr_preload(GFP_KERNEL);
 	spin_lock(&fc->lock);
+	idr = rcu_dereference_protected(fc->backing_files_map,
+					lockdep_is_held(&fc->lock));
 	/* FIXME: xarray might be space inefficient */
-	id = idr_alloc_cyclic(&fc->backing_files_map, fb, 1, 0, GFP_ATOMIC);
+	id = idr_alloc_cyclic(idr, fb, 1, 0, GFP_ATOMIC);
 	spin_unlock(&fc->lock);
 	idr_preload_end();
 
@@ -55,10 +65,13 @@ static int fuse_backing_id_alloc(struct fuse_conn *fc, struct fuse_backing *fb)
 static struct fuse_backing *fuse_backing_id_remove(struct fuse_conn *fc,
 						   int id)
 {
+	struct idr *idr;
 	struct fuse_backing *fb;
 
 	spin_lock(&fc->lock);
-	fb = idr_remove(&fc->backing_files_map, id);
+	idr = rcu_dereference_protected(fc->backing_files_map,
+					lockdep_is_held(&fc->lock));
+	fb = idr_remove(idr, id);
 	spin_unlock(&fc->lock);
 
 	return fb;
@@ -75,8 +88,13 @@ static int fuse_backing_id_free(int id, void *p, void *data)
 
 void fuse_backing_files_free(struct fuse_conn *fc)
 {
-	idr_for_each(&fc->backing_files_map, fuse_backing_id_free, NULL);
-	idr_destroy(&fc->backing_files_map);
+	struct idr *idr = rcu_dereference_protected(fc->backing_files_map, 1);
+
+	if (idr) {
+		idr_for_each(idr, fuse_backing_id_free, NULL);
+		idr_destroy(idr);
+		kfree(idr);
+	}
 }
 
 int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
@@ -166,12 +184,57 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
 	return err;
 }
 
+int fuse_backing_close_all(struct fuse_conn *fc)
+{
+	struct idr *old_idr, *new_idr;
+	struct fuse_backing *fb;
+	int id;
+
+	if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	new_idr = kzalloc(sizeof(*new_idr), GFP_KERNEL);
+	if (!new_idr)
+		return -ENOMEM;
+
+	idr_init(new_idr);
+
+	/*
+	 * Atomically exchange the old IDR with a new empty one under lock.
+	 * This avoids long lock hold times and races with concurrent
+	 * open/close operations.
+	 */
+	spin_lock(&fc->lock);
+	old_idr = rcu_dereference_protected(fc->backing_files_map,
+					    lockdep_is_held(&fc->lock));
+	rcu_assign_pointer(fc->backing_files_map, new_idr);
+	spin_unlock(&fc->lock);
+
+	/*
+	 * Ensure all concurrent RCU readers complete before releasing backing
+	 * files, so any in-flight lookups can safely take references.
+	 */
+	synchronize_rcu();
+
+	if (old_idr) {
+		idr_for_each_entry(old_idr, fb, id)
+			fuse_backing_put(fb);
+
+		idr_destroy(old_idr);
+		kfree(old_idr);
+	}
+
+	return 0;
+}
+
 struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc, int backing_id)
 {
+	struct idr *idr;
 	struct fuse_backing *fb;
 
 	rcu_read_lock();
-	fb = idr_find(&fc->backing_files_map, backing_id);
+	idr = rcu_dereference(fc->backing_files_map);
+	fb = idr ? idr_find(idr, backing_id) : NULL;
 	fb = fuse_backing_get(fb);
 	rcu_read_unlock();
 
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 6d59cbc877c6..f05d55302598 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2654,6 +2654,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
 	return fuse_backing_close(fud->fc, backing_id);
 }
 
+static long fuse_dev_ioctl_backing_close_all(struct file *file)
+{
+	struct fuse_dev *fud = fuse_get_dev(file);
+
+	if (IS_ERR(fud))
+		return PTR_ERR(fud);
+
+	if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
+		return -EOPNOTSUPP;
+
+	return fuse_backing_close_all(fud->fc);
+}
+
 static long fuse_dev_ioctl_sync_init(struct file *file)
 {
 	int err = -EINVAL;
@@ -2682,6 +2695,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
 	case FUSE_DEV_IOC_BACKING_CLOSE:
 		return fuse_dev_ioctl_backing_close(file, argp);
 
+	case FUSE_DEV_IOC_BACKING_CLOSE_ALL:
+		return fuse_dev_ioctl_backing_close_all(file);
+
 	case FUSE_DEV_IOC_SYNC_INIT:
 		return fuse_dev_ioctl_sync_init(file);
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7f16049387d1..f45c5042e31a 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -979,7 +979,7 @@ struct fuse_conn {
 
 #ifdef CONFIG_FUSE_PASSTHROUGH
 	/** IDR for backing files ids */
-	struct idr backing_files_map;
+	struct idr __rcu *backing_files_map;
 #endif
 
 #ifdef CONFIG_FUSE_IO_URING
@@ -1569,10 +1569,11 @@ static inline struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc,
 }
 #endif
 
-void fuse_backing_files_init(struct fuse_conn *fc);
+int fuse_backing_files_init(struct fuse_conn *fc);
 void fuse_backing_files_free(struct fuse_conn *fc);
 int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map);
 int fuse_backing_close(struct fuse_conn *fc, int backing_id);
+int fuse_backing_close_all(struct fuse_conn *fc);
 
 /* passthrough.c */
 static inline struct fuse_backing *fuse_inode_backing(struct fuse_inode *fi)
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 819e50d66622..b63a067d50f8 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1001,9 +1001,6 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 	fc->name_max = FUSE_NAME_LOW_MAX;
 	fc->timeout.req_timeout = 0;
 
-	if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
-		fuse_backing_files_init(fc);
-
 	INIT_LIST_HEAD(&fc->mounts);
 	list_add(&fm->fc_entry, &fc->mounts);
 	fm->fc = fc;
@@ -1439,9 +1436,11 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			    arg->max_stack_depth > 0 &&
 			    arg->max_stack_depth <= FILESYSTEM_MAX_STACK_DEPTH &&
 			    !(flags & FUSE_WRITEBACK_CACHE))  {
-				fc->passthrough = 1;
-				fc->max_stack_depth = arg->max_stack_depth;
-				fm->sb->s_stack_depth = arg->max_stack_depth;
+				if (fuse_backing_files_init(fc) == 0) {
+					fc->passthrough = 1;
+					fc->max_stack_depth = arg->max_stack_depth;
+					fm->sb->s_stack_depth = arg->max_stack_depth;
+				}
 			}
 			if (flags & FUSE_NO_EXPORT_SUPPORT)
 				fm->sb->s_export_op = &fuse_export_fid_operations;
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index c13e1f9a2f12..e4ff28a4ff40 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -1139,6 +1139,7 @@ struct fuse_backing_map {
 					     struct fuse_backing_map)
 #define FUSE_DEV_IOC_BACKING_CLOSE	_IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t)
 #define FUSE_DEV_IOC_SYNC_INIT		_IO(FUSE_DEV_IOC_MAGIC, 3)
+#define FUSE_DEV_IOC_BACKING_CLOSE_ALL	_IO(FUSE_DEV_IOC_MAGIC, 4)
 
 struct fuse_lseek_in {
 	uint64_t	fh;
-- 
2.41.0

On Sun, Jan 18, 2026 at 12:47 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>
>
>
> On 1/18/26 1:00 AM, Amir Goldstein wrote:
> > On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> >>
> >>
> >>
> >> On 1/16/26 11:39 PM, Amir Goldstein wrote:
> >>> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> >>>>
> >>>> To simplify crash recovery and reduce performance impact, backing_ids
> >>>> are not persisted across daemon restarts. After crash recovery, this
> >>>> may lead to resource leaks if backing file resources are not properly
> >>>> cleaned up.
> >>>>
> >>>> Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
> >>>> and put backing files. When the FUSE daemon restarts, it can use this
> >>>> ioctl to cleanup all backing file resources.
> >>>>
> >>>> Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
> >>>> ---
> >>>>    fs/fuse/backing.c         | 19 +++++++++++++++++++
> >>>>    fs/fuse/dev.c             | 16 ++++++++++++++++
> >>>>    fs/fuse/fuse_i.h          |  1 +
> >>>>    include/uapi/linux/fuse.h |  1 +
> >>>>    4 files changed, 37 insertions(+)
> >>>>
> >>>> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> >>>> index 4afda419dd14..e93d797a2cde 100644
> >>>> --- a/fs/fuse/backing.c
> >>>> +++ b/fs/fuse/backing.c
> >>>> @@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
> >>>>           return err;
> >>>>    }
> >>>>
> >>>> +static int fuse_backing_close_one(int id, void *p, void *data)
> >>>> +{
> >>>> +       struct fuse_conn *fc = data;
> >>>> +
> >>>> +       fuse_backing_close(fc, id);
> >>>> +
> >>>> +       return 0;
> >>>> +}
> >>>> +
> >>>> +int fuse_backing_close_all(struct fuse_conn *fc)
> >>>> +{
> >>>> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> >>>> +               return -EPERM;
> >>>> +
> >>>> +       idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
> >>>> +
> >>>> +       return 0;
> >>>> +}
> >>>> +
> >>>
> >>> This is not safe and not efficient.
> >>> For safety from racing with _open/_close, iteration needs at least
> >>> rcu_read_lock(),
> >>
> >> Yes, you're absolutely right. Additionally, calling idr_remove within
> >> idr_for_each maybe presents safety risks.
> >>
> >>> but I think it will be much more efficient to zap the entire map with
> >>> fuse_backing_files_free()/fuse_backing_files_init().
> >>>
> >>> This of course needs to be synchronized with concurrent _open/_close/_lookup.
> >>> This could be done by making c->backing_files_map a struct idr __rcu *
> >>> and replace the old and new backing_files_map under spin_lock(&fc->lock);
> >>>
> >>> Then you can call fuse_backing_files_free() on the old backing_files_map
> >>> without a lock.
> >>>
> >>> As a side note, fuse_backing_files_free() iteration looks like it may need
> >>> cond_resched() if there are a LOT of backing ids, but I am not sure and
> >>> this is orthogonal to your change.
> >>>
> >>> Thanks,
> >>> Amir.
> >>>
> >>>
> >>
> >> Thank you for your helpful suggestions. However, it cannot use
> >> fuse_backing_files_free() in the close_all implementation because it
> >> directly frees backing files without respecting reference counts. This
> >> function requires that no one is actively using the backing file (it
> >> even has WARN_ON_ONCE(refcount_read(&fb->count) != 1)), which cannot be
> >> guaranteed after a crash recovery scenario where backing files may still
> >> be in use.
> >
> > Right.
> >
> >>
> >> Instead, the implementation uses fuse_backing_put() to safely decrement
> >> the reference count and allow the backing file to be freed when no
> >> longer in use.
> >
> > OK.
> >
> >>
> >> Additionally, the implementation addresses two race conditions:
> >>
> >> - Race between idr_for_each and lookup: Uses synchronize_rcu() to ensure
> >> all concurrent RCU readers (i.e., in-flight fuse_backing_lookup() calls)
> >> complete before releasing backing files, preventing use-after-free issues.
> >
> > Almost. See below.
> >
> >>
> >> - Race with open/close operations: Uses fc->lock to atomically swap the
> >> old and new IDR maps, ensuring consistency with concurrent
> >> fuse_backing_open() and fuse_backing_close() operations.
> >>
> >> This approach provides the same as the RCU pointer suggestion, but with
> >> less code and no changes to the struct fuse_conn data structures.
> >>
> >> I've updated it and verified the implementation. Could you please review it?
> >>
> >>
> >> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> >> index 4afda419dd14..047d373684f9 100644
> >> --- a/fs/fuse/backing.c
> >> +++ b/fs/fuse/backing.c
> >> @@ -166,6 +166,45 @@ int fuse_backing_close(struct fuse_conn *fc, int
> >> backing_id)
> >>           return err;
> >>    }
> >>
> >> +static int fuse_backing_release_one(int id, void *p, void *data)
> >> +{
> >> +       struct fuse_backing *fb = p;
> >> +
> >> +       fuse_backing_put(fb);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +int fuse_backing_close_all(struct fuse_conn *fc)
> >> +{
> >> +       struct idr old_map;
> >> +
> >> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> >> +               return -EPERM;
> >> +
> >> +       /*
> >> +        * Swap out the old backing_files_map with a new empty one under
> >> lock,
> >> +        * then release all backing files outside the lock. This avoids long
> >> +        * lock hold times and potential races with concurrent open/close
> >> +        * operations.
> >> +        */
> >> +       idr_init(&old_map);
> >> +       spin_lock(&fc->lock);
> >> +       swap(fc->backing_files_map, old_map);
> >> +       spin_unlock(&fc->lock);
> >> +
> >> +       /*
> >> +        * Ensure all concurrent RCU readers complete before releasing
> >> backing
> >> +        * files, so any in-flight lookups can safely take references.
> >> +        */
> >> +       synchronize_rcu();
> >> +
> >> +       idr_for_each(&old_map, fuse_backing_release_one, NULL);
> >> +       idr_destroy(&old_map);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >
> > That's almost safe but not enough.
> > This lookup code is not safe against the swap():
> >
> >    rcu_read_lock();
> >    fb = idr_find(&fc->backing_files_map, backing_id);
> >
> > That is the reason you need to make fc->backing_files_map
> > an rcu referenced ptr.
> >
> > Instead of swap() you use xchg() to atomically exchange the
> > old and new struct idr pointers and for lookup:
> >
> >    rcu_read_lock();
> >    fb = idr_find(rcu_dereference(fc->backing_files_map), backing_id);
> >
> > Thanks,
> > Amir.
> >
> >
>
> Yes, swap() isn't atomic, it's just copying structs, so it's not safe
> when racing with lookup.
>
> I've updated the version to make fc->backing_files_map an rcu referenced
> ptr. Please review the attached patch.

You can also use rcu_replace_pointer() to swap old_idr <-> new_idr,
but otherwise the patch looks fine to me.

Thanks,
Amir.

On Sun, Jan 18, 2026 at 6:07 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Sun, Jan 18, 2026 at 12:47 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> >
> >
> >
> > On 1/18/26 1:00 AM, Amir Goldstein wrote:
> > > On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> > >>
> > >>
> > >>
> > >> On 1/16/26 11:39 PM, Amir Goldstein wrote:
> > >>> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> > >>>>
> > >>>> To simplify crash recovery and reduce performance impact, backing_ids
> > >>>> are not persisted across daemon restarts. After crash recovery, this
> > >>>> may lead to resource leaks if backing file resources are not properly
> > >>>> cleaned up.
> > >>>>
> > >>>> Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
> > >>>> and put backing files. When the FUSE daemon restarts, it can use this
> > >>>> ioctl to cleanup all backing file resources.
> > >>>>
> > >>>> Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
> > >>>> ---
> > >>>>    fs/fuse/backing.c         | 19 +++++++++++++++++++
> > >>>>    fs/fuse/dev.c             | 16 ++++++++++++++++
> > >>>>    fs/fuse/fuse_i.h          |  1 +
> > >>>>    include/uapi/linux/fuse.h |  1 +
> > >>>>    4 files changed, 37 insertions(+)
> > >>>>
> > >>>> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> > >>>> index 4afda419dd14..e93d797a2cde 100644
> > >>>> --- a/fs/fuse/backing.c
> > >>>> +++ b/fs/fuse/backing.c
> > >>>> @@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
> > >>>>           return err;
> > >>>>    }
> > >>>>
> > >>>> +static int fuse_backing_close_one(int id, void *p, void *data)
> > >>>> +{
> > >>>> +       struct fuse_conn *fc = data;
> > >>>> +
> > >>>> +       fuse_backing_close(fc, id);
> > >>>> +
> > >>>> +       return 0;
> > >>>> +}
> > >>>> +
> > >>>> +int fuse_backing_close_all(struct fuse_conn *fc)
> > >>>> +{
> > >>>> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> > >>>> +               return -EPERM;
> > >>>> +
> > >>>> +       idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
> > >>>> +
> > >>>> +       return 0;
> > >>>> +}
> > >>>> +
> > >>>
> > >>> This is not safe and not efficient.
> > >>> For safety from racing with _open/_close, iteration needs at least
> > >>> rcu_read_lock(),
> > >>
> > >> Yes, you're absolutely right. Additionally, calling idr_remove within
> > >> idr_for_each maybe presents safety risks.
> > >>
> > >>> but I think it will be much more efficient to zap the entire map with
> > >>> fuse_backing_files_free()/fuse_backing_files_init().
> > >>>
> > >>> This of course needs to be synchronized with concurrent _open/_close/_lookup.
> > >>> This could be done by making c->backing_files_map a struct idr __rcu *
> > >>> and replace the old and new backing_files_map under spin_lock(&fc->lock);
> > >>>
> > >>> Then you can call fuse_backing_files_free() on the old backing_files_map
> > >>> without a lock.
> > >>>
> > >>> As a side note, fuse_backing_files_free() iteration looks like it may need
> > >>> cond_resched() if there are a LOT of backing ids, but I am not sure and
> > >>> this is orthogonal to your change.
> > >>>
> > >>> Thanks,
> > >>> Amir.
> > >>>
> > >>>
> > >>
> > >> Thank you for your helpful suggestions. However, it cannot use
> > >> fuse_backing_files_free() in the close_all implementation because it
> > >> directly frees backing files without respecting reference counts. This
> > >> function requires that no one is actively using the backing file (it
> > >> even has WARN_ON_ONCE(refcount_read(&fb->count) != 1)), which cannot be
> > >> guaranteed after a crash recovery scenario where backing files may still
> > >> be in use.
> > >
> > > Right.
> > >
> > >>
> > >> Instead, the implementation uses fuse_backing_put() to safely decrement
> > >> the reference count and allow the backing file to be freed when no
> > >> longer in use.
> > >
> > > OK.
> > >
> > >>
> > >> Additionally, the implementation addresses two race conditions:
> > >>
> > >> - Race between idr_for_each and lookup: Uses synchronize_rcu() to ensure
> > >> all concurrent RCU readers (i.e., in-flight fuse_backing_lookup() calls)
> > >> complete before releasing backing files, preventing use-after-free issues.
> > >
> > > Almost. See below.
> > >
> > >>
> > >> - Race with open/close operations: Uses fc->lock to atomically swap the
> > >> old and new IDR maps, ensuring consistency with concurrent
> > >> fuse_backing_open() and fuse_backing_close() operations.
> > >>
> > >> This approach provides the same as the RCU pointer suggestion, but with
> > >> less code and no changes to the struct fuse_conn data structures.
> > >>
> > >> I've updated it and verified the implementation. Could you please review it?
> > >>
> > >>
> > >> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> > >> index 4afda419dd14..047d373684f9 100644
> > >> --- a/fs/fuse/backing.c
> > >> +++ b/fs/fuse/backing.c
> > >> @@ -166,6 +166,45 @@ int fuse_backing_close(struct fuse_conn *fc, int
> > >> backing_id)
> > >>           return err;
> > >>    }
> > >>
> > >> +static int fuse_backing_release_one(int id, void *p, void *data)
> > >> +{
> > >> +       struct fuse_backing *fb = p;
> > >> +
> > >> +       fuse_backing_put(fb);
> > >> +
> > >> +       return 0;
> > >> +}
> > >> +
> > >> +int fuse_backing_close_all(struct fuse_conn *fc)
> > >> +{
> > >> +       struct idr old_map;
> > >> +
> > >> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> > >> +               return -EPERM;
> > >> +
> > >> +       /*
> > >> +        * Swap out the old backing_files_map with a new empty one under
> > >> lock,
> > >> +        * then release all backing files outside the lock. This avoids long
> > >> +        * lock hold times and potential races with concurrent open/close
> > >> +        * operations.
> > >> +        */
> > >> +       idr_init(&old_map);
> > >> +       spin_lock(&fc->lock);
> > >> +       swap(fc->backing_files_map, old_map);
> > >> +       spin_unlock(&fc->lock);
> > >> +
> > >> +       /*
> > >> +        * Ensure all concurrent RCU readers complete before releasing
> > >> backing
> > >> +        * files, so any in-flight lookups can safely take references.
> > >> +        */
> > >> +       synchronize_rcu();
> > >> +
> > >> +       idr_for_each(&old_map, fuse_backing_release_one, NULL);
> > >> +       idr_destroy(&old_map);
> > >> +
> > >> +       return 0;
> > >> +}
> > >> +
> > >
> > > That's almost safe but not enough.
> > > This lookup code is not safe against the swap():
> > >
> > >    rcu_read_lock();
> > >    fb = idr_find(&fc->backing_files_map, backing_id);
> > >
> > > That is the reason you need to make fc->backing_files_map
> > > an rcu referenced ptr.
> > >
> > > Instead of swap() you use xchg() to atomically exchange the
> > > old and new struct idr pointers and for lookup:
> > >
> > >    rcu_read_lock();
> > >    fb = idr_find(rcu_dereference(fc->backing_files_map), backing_id);
> > >
> > > Thanks,
> > > Amir.
> > >
> > >
> >
> > Yes, swap() isn't atomic, it's just copying structs, so it's not safe
> > when racing with lookup.
> >
> > I've updated the version to make fc->backing_files_map an rcu referenced
> > ptr. Please review the attached patch.
>
> You can also use rcu_replace_pointer() to swap old_idr <-> new_idr,
> but otherwise the patch looks fine to me.
>

Feel free to add
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

On 1/19/26 1:08 AM, Amir Goldstein wrote:
> On Sun, Jan 18, 2026 at 6:07 PM Amir Goldstein <amir73il@gmail.com> wrote:
>>
>> On Sun, Jan 18, 2026 at 12:47 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>>
>>>
>>>
>>> On 1/18/26 1:00 AM, Amir Goldstein wrote:
>>>> On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 1/16/26 11:39 PM, Amir Goldstein wrote:
>>>>>> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>>>>>>
>>>>>>> To simplify crash recovery and reduce performance impact, backing_ids
>>>>>>> are not persisted across daemon restarts. After crash recovery, this
>>>>>>> may lead to resource leaks if backing file resources are not properly
>>>>>>> cleaned up.
>>>>>>>
>>>>>>> Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
>>>>>>> and put backing files. When the FUSE daemon restarts, it can use this
>>>>>>> ioctl to cleanup all backing file resources.
>>>>>>>
>>>>>>> Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
>>>>>>> ---
>>>>>>>     fs/fuse/backing.c         | 19 +++++++++++++++++++
>>>>>>>     fs/fuse/dev.c             | 16 ++++++++++++++++
>>>>>>>     fs/fuse/fuse_i.h          |  1 +
>>>>>>>     include/uapi/linux/fuse.h |  1 +
>>>>>>>     4 files changed, 37 insertions(+)
>>>>>>>
>>>>>>> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
>>>>>>> index 4afda419dd14..e93d797a2cde 100644
>>>>>>> --- a/fs/fuse/backing.c
>>>>>>> +++ b/fs/fuse/backing.c
>>>>>>> @@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
>>>>>>>            return err;
>>>>>>>     }
>>>>>>>
>>>>>>> +static int fuse_backing_close_one(int id, void *p, void *data)
>>>>>>> +{
>>>>>>> +       struct fuse_conn *fc = data;
>>>>>>> +
>>>>>>> +       fuse_backing_close(fc, id);
>>>>>>> +
>>>>>>> +       return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +int fuse_backing_close_all(struct fuse_conn *fc)
>>>>>>> +{
>>>>>>> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
>>>>>>> +               return -EPERM;
>>>>>>> +
>>>>>>> +       idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
>>>>>>> +
>>>>>>> +       return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>
>>>>>> This is not safe and not efficient.
>>>>>> For safety from racing with _open/_close, iteration needs at least
>>>>>> rcu_read_lock(),
>>>>>
>>>>> Yes, you're absolutely right. Additionally, calling idr_remove within
>>>>> idr_for_each maybe presents safety risks.
>>>>>
>>>>>> but I think it will be much more efficient to zap the entire map with
>>>>>> fuse_backing_files_free()/fuse_backing_files_init().
>>>>>>
>>>>>> This of course needs to be synchronized with concurrent _open/_close/_lookup.
>>>>>> This could be done by making c->backing_files_map a struct idr __rcu *
>>>>>> and replace the old and new backing_files_map under spin_lock(&fc->lock);
>>>>>>
>>>>>> Then you can call fuse_backing_files_free() on the old backing_files_map
>>>>>> without a lock.
>>>>>>
>>>>>> As a side note, fuse_backing_files_free() iteration looks like it may need
>>>>>> cond_resched() if there are a LOT of backing ids, but I am not sure and
>>>>>> this is orthogonal to your change.
>>>>>>
>>>>>> Thanks,
>>>>>> Amir.
>>>>>>
>>>>>>
>>>>>
>>>>> Thank you for your helpful suggestions. However, it cannot use
>>>>> fuse_backing_files_free() in the close_all implementation because it
>>>>> directly frees backing files without respecting reference counts. This
>>>>> function requires that no one is actively using the backing file (it
>>>>> even has WARN_ON_ONCE(refcount_read(&fb->count) != 1)), which cannot be
>>>>> guaranteed after a crash recovery scenario where backing files may still
>>>>> be in use.
>>>>
>>>> Right.
>>>>
>>>>>
>>>>> Instead, the implementation uses fuse_backing_put() to safely decrement
>>>>> the reference count and allow the backing file to be freed when no
>>>>> longer in use.
>>>>
>>>> OK.
>>>>
>>>>>
>>>>> Additionally, the implementation addresses two race conditions:
>>>>>
>>>>> - Race between idr_for_each and lookup: Uses synchronize_rcu() to ensure
>>>>> all concurrent RCU readers (i.e., in-flight fuse_backing_lookup() calls)
>>>>> complete before releasing backing files, preventing use-after-free issues.
>>>>
>>>> Almost. See below.
>>>>
>>>>>
>>>>> - Race with open/close operations: Uses fc->lock to atomically swap the
>>>>> old and new IDR maps, ensuring consistency with concurrent
>>>>> fuse_backing_open() and fuse_backing_close() operations.
>>>>>
>>>>> This approach provides the same as the RCU pointer suggestion, but with
>>>>> less code and no changes to the struct fuse_conn data structures.
>>>>>
>>>>> I've updated it and verified the implementation. Could you please review it?
>>>>>
>>>>>
>>>>> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
>>>>> index 4afda419dd14..047d373684f9 100644
>>>>> --- a/fs/fuse/backing.c
>>>>> +++ b/fs/fuse/backing.c
>>>>> @@ -166,6 +166,45 @@ int fuse_backing_close(struct fuse_conn *fc, int
>>>>> backing_id)
>>>>>            return err;
>>>>>     }
>>>>>
>>>>> +static int fuse_backing_release_one(int id, void *p, void *data)
>>>>> +{
>>>>> +       struct fuse_backing *fb = p;
>>>>> +
>>>>> +       fuse_backing_put(fb);
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>> +int fuse_backing_close_all(struct fuse_conn *fc)
>>>>> +{
>>>>> +       struct idr old_map;
>>>>> +
>>>>> +       if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
>>>>> +               return -EPERM;
>>>>> +
>>>>> +       /*
>>>>> +        * Swap out the old backing_files_map with a new empty one under
>>>>> lock,
>>>>> +        * then release all backing files outside the lock. This avoids long
>>>>> +        * lock hold times and potential races with concurrent open/close
>>>>> +        * operations.
>>>>> +        */
>>>>> +       idr_init(&old_map);
>>>>> +       spin_lock(&fc->lock);
>>>>> +       swap(fc->backing_files_map, old_map);
>>>>> +       spin_unlock(&fc->lock);
>>>>> +
>>>>> +       /*
>>>>> +        * Ensure all concurrent RCU readers complete before releasing
>>>>> backing
>>>>> +        * files, so any in-flight lookups can safely take references.
>>>>> +        */
>>>>> +       synchronize_rcu();
>>>>> +
>>>>> +       idr_for_each(&old_map, fuse_backing_release_one, NULL);
>>>>> +       idr_destroy(&old_map);
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>
>>>> That's almost safe but not enough.
>>>> This lookup code is not safe against the swap():
>>>>
>>>>     rcu_read_lock();
>>>>     fb = idr_find(&fc->backing_files_map, backing_id);
>>>>
>>>> That is the reason you need to make fc->backing_files_map
>>>> an rcu referenced ptr.
>>>>
>>>> Instead of swap() you use xchg() to atomically exchange the
>>>> old and new struct idr pointers and for lookup:
>>>>
>>>>     rcu_read_lock();
>>>>     fb = idr_find(rcu_dereference(fc->backing_files_map), backing_id);
>>>>
>>>> Thanks,
>>>> Amir.
>>>>
>>>>
>>>
>>> Yes, swap() isn't atomic, it's just copying structs, so it's not safe
>>> when racing with lookup.
>>>
>>> I've updated the version to make fc->backing_files_map an rcu referenced
>>> ptr. Please review the attached patch.
>>
>> You can also use rcu_replace_pointer() to swap old_idr <-> new_idr,
>> but otherwise the patch looks fine to me.
>>
> 
> Feel free to add
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> Thanks,
> Amir.
> 
> 

Ok, agree. Using rcu_replace_pointer is more concise.

Thanks,
Chunsheng Luo