[PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Zhiwei Liang via B4 Relay]

From: Zhiwei Liang <zhiwei.liang@zliang.me>

When a suspend starts during FITRIM, fstrim can fail with:

  fstrim: /: FITRIM ioctl failed: Unknown error 512

On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
fstrim.service runs were in progress when an automatic suspend started.
All three failed this way and left the filesystem untrimmed.

Since commit 69313850dce3 ("btrfs: add cancellation points to trim
loops"), the trim loops return -ERESTARTSYS when
btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.

-ERESTARTSYS is only valid while a signal is pending, because only the
signal handling on syscall exit turns it into a restart. But
freeze_processes() first sets pm_freezing, which makes freezing() true
for every user task. Then it walks the task list and sends each task a
fake signal. The trim loops check btrfs_trim_interrupted() after every
extent. If a check falls in this window, the loop returns -ERESTARTSYS
with no signal pending. Nothing restarts the syscall. On that laptop,
with a few thousand threads, freezing user space took 2 to 5 ms in each
of the three suspends above, and all three runs hit this window.

Make btrfs_trim_interrupted() treat a freeze as an interruption only
once the fake signal has arrived, i.e. when
freezing(current) && signal_pending(current). freeze_processes() sends
that signal in the same call. The trim loop stops at the next check
after it, so suspend is not delayed.

The restarted syscall re-reads struct fstrim_range from user memory.
When the device phase is interrupted, btrfs_trim_fs() has already stored
the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
the struct back unconditionally. The restart is then asked to trim
[start, start + trimmed) instead of the requested range, or fails with
-EINVAL when nothing was counted yet. Skip the copy_to_user() for
-ERESTARTSYS. In all other cases the partial count is still copied back.

Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
during a trim leaked errno 512, and 10 of 10 restarts after a
device-phase interruption failed with -EINVAL. With the patch, all 30
runs restarted and trimmed the full amount.

Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
CC: stable@vger.kernel.org # 5.15+
Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
---
 fs/btrfs/free-space-cache.h | 3 ++-
 fs/btrfs/ioctl.c            | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
index e22443598b..cba2f5fd6b 100644
--- a/fs/btrfs/free-space-cache.h
+++ b/fs/btrfs/free-space-cache.h
@@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
 
 static inline bool btrfs_trim_interrupted(void)
 {
-	return fatal_signal_pending(current) || freezing(current);
+	return fatal_signal_pending(current) ||
+	       (freezing(current) && signal_pending(current));
 }
 
 /*
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ac070c745b..b02c59cbb0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
 	range.minlen = max(range.minlen, minlen);
 	ret = btrfs_trim_fs(fs_info, &range);
 
+	/*
+	 * A restart re-reads the range from user space, so do not clobber it
+	 * with the partial len the rerun would otherwise take as its input.
+	 */
+	if (ret == -ERESTARTSYS)
+		return ret;
+
 	if (copy_to_user(arg, &range, sizeof(range)))
 		return -EFAULT;
 

---
base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13

Best regards,
-- 
Zhiwei Liang <zhiwei.liang@zliang.me>

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Qu Wenruo]

在 2026/10/28 17:27, Zhiwei Liang via B4 Relay 写道:
> From: Zhiwei Liang <zhiwei.liang@zliang.me>
> 
> When a suspend starts during FITRIM, fstrim can fail with:
> 
>    fstrim: /: FITRIM ioctl failed: Unknown error 512
> 
> On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
> fstrim.service runs were in progress when an automatic suspend started.
> All three failed this way and left the filesystem untrimmed.
> 
> Since commit 69313850dce3 ("btrfs: add cancellation points to trim
> loops"), the trim loops return -ERESTARTSYS when
> btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
> to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
> properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.
> 
> -ERESTARTSYS is only valid while a signal is pending, because only the
> signal handling on syscall exit turns it into a restart. But
> freeze_processes() first sets pm_freezing, which makes freezing() true
> for every user task. Then it walks the task list and sends each task a
> fake signal. The trim loops check btrfs_trim_interrupted() after every
> extent. If a check falls in this window, the loop returns -ERESTARTSYS
> with no signal pending. Nothing restarts the syscall. On that laptop,
> with a few thousand threads, freezing user space took 2 to 5 ms in each
> of the three suspends above, and all three runs hit this window.
> 
> Make btrfs_trim_interrupted() treat a freeze as an interruption only
> once the fake signal has arrived, i.e. when
> freezing(current) && signal_pending(current). freeze_processes() sends
> that signal in the same call. The trim loop stops at the next check
> after it, so suspend is not delayed.
> 
> The restarted syscall re-reads struct fstrim_range from user memory.
> When the device phase is interrupted, btrfs_trim_fs() has already stored
> the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
> the struct back unconditionally. The restart is then asked to trim
> [start, start + trimmed) instead of the requested range, or fails with
> -EINVAL when nothing was counted yet. Skip the copy_to_user() for
> -ERESTARTSYS. In all other cases the partial count is still copied back.
> 
> Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
> during a trim leaked errno 512, and 10 of 10 restarts after a
> device-phase interruption failed with -EINVAL. With the patch, all 30
> runs restarted and trimmed the full amount.
> 
> Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
> CC: stable@vger.kernel.org # 5.15+
> Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
> ---
>   fs/btrfs/free-space-cache.h | 3 ++-
>   fs/btrfs/ioctl.c            | 7 +++++++
>   2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
> index e22443598b..cba2f5fd6b 100644
> --- a/fs/btrfs/free-space-cache.h
> +++ b/fs/btrfs/free-space-cache.h
> @@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
>   
>   static inline bool btrfs_trim_interrupted(void)
>   {
> -	return fatal_signal_pending(current) || freezing(current);
> +	return fatal_signal_pending(current) ||
> +	       (freezing(current) && signal_pending(current));

What if pm freeze has began but a real signal is also pending before the 
fake pm signal?

>   }
>   
>   /*
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index ac070c745b..b02c59cbb0 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
>   	range.minlen = max(range.minlen, minlen);
>   	ret = btrfs_trim_fs(fs_info, &range);
>   
> +	/*
> +	 * A restart re-reads the range from user space, so do not clobber it
> +	 * with the partial len the rerun would otherwise take as its input.
> +	 */
> +	if (ret == -ERESTARTSYS)
> +		return ret;
> +
>   	if (copy_to_user(arg, &range, sizeof(range)))
>   		return -EFAULT;
>   
> 
> ---
> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
> change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13
> 
> Best regards,

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[zhiwei.liang]

[-- Attachment #1.1: Type: text/plain, Size: 947 bytes --]

> >   static inline bool btrfs_trim_interrupted(void)
> >   {
> > -	return fatal_signal_pending(current) || freezing(current);
> > +	return fatal_signal_pending(current) ||
> > +	       (freezing(current) && signal_pending(current));
> 

> What if pm freeze has began but a real signal is also pending before the
> fake pm signal?

I think when a real signal is already pending as the freeze begins, both
freezing(current) and signal_pending(current) are true, so the loop stops
with -ERESTARTSYS, same as for any interrupted syscall. On the way back
to user mode the task is parked for the suspend first, and the real
signal is handled after thaw.

That said, I agree the ext4/xfs approach is the better fix. Luca's series
doing that supersedes this patch.

One correction to my tags: 69313850dce3 was never backported to 5.15 or
6.1, only 6.6.y and later have it, so the stable range should have been
6.6+ rather than 5.15+.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 343 bytes --]

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Luca Stefani]

On Wed, 28 Oct 2026 at 07:57, Zhiwei Liang via B4 Relay
<devnull+zhiwei.liang.zliang.me@kernel.org> wrote:
>
> From: Zhiwei Liang <zhiwei.liang@zliang.me>
>
> When a suspend starts during FITRIM, fstrim can fail with:
>
>   fstrim: /: FITRIM ioctl failed: Unknown error 512
>
> On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
> fstrim.service runs were in progress when an automatic suspend started.
> All three failed this way and left the filesystem untrimmed.
>
> Since commit 69313850dce3 ("btrfs: add cancellation points to trim
> loops"), the trim loops return -ERESTARTSYS when
> btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
> to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
> properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.
>
> -ERESTARTSYS is only valid while a signal is pending, because only the
> signal handling on syscall exit turns it into a restart. But
> freeze_processes() first sets pm_freezing, which makes freezing() true
> for every user task. Then it walks the task list and sends each task a
> fake signal. The trim loops check btrfs_trim_interrupted() after every
> extent. If a check falls in this window, the loop returns -ERESTARTSYS
> with no signal pending. Nothing restarts the syscall. On that laptop,
> with a few thousand threads, freezing user space took 2 to 5 ms in each
> of the three suspends above, and all three runs hit this window.
>
> Make btrfs_trim_interrupted() treat a freeze as an interruption only
> once the fake signal has arrived, i.e. when
> freezing(current) && signal_pending(current). freeze_processes() sends
> that signal in the same call. The trim loop stops at the next check
> after it, so suspend is not delayed.
Looking at how ext4 handles this scenario it seems to me they don't set
the error code to -ERESTARTSYS and (from a quick glance) return the
bytes discarded to the caller.
And like before they continue the discard (which will skip/fail again) until
there's nothing to be done.

I don't know what the intention of btrfs: handle user interrupt
properly in btrfs_trim_fs()
was, maybe we should just swallow the ERESTARTSYS case in those scenarios.
>
> The restarted syscall re-reads struct fstrim_range from user memory.
> When the device phase is interrupted, btrfs_trim_fs() has already stored
> the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
> the struct back unconditionally. The restart is then asked to trim
> [start, start + trimmed) instead of the requested range, or fails with
> -EINVAL when nothing was counted yet. Skip the copy_to_user() for
> -ERESTARTSYS. In all other cases the partial count is still copied back.
>
> Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
> during a trim leaked errno 512, and 10 of 10 restarts after a
> device-phase interruption failed with -EINVAL. With the patch, all 30
> runs restarted and trimmed the full amount.
>
> Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
> CC: stable@vger.kernel.org # 5.15+
> Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
> ---
>  fs/btrfs/free-space-cache.h | 3 ++-
>  fs/btrfs/ioctl.c            | 7 +++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
> index e22443598b..cba2f5fd6b 100644
> --- a/fs/btrfs/free-space-cache.h
> +++ b/fs/btrfs/free-space-cache.h
> @@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
>
>  static inline bool btrfs_trim_interrupted(void)
>  {
> -       return fatal_signal_pending(current) || freezing(current);
> +       return fatal_signal_pending(current) ||
> +              (freezing(current) && signal_pending(current));
>  }
>
>  /*
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index ac070c745b..b02c59cbb0 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
>         range.minlen = max(range.minlen, minlen);
>         ret = btrfs_trim_fs(fs_info, &range);
>
> +       /*
> +        * A restart re-reads the range from user space, so do not clobber it
> +        * with the partial len the rerun would otherwise take as its input.
> +        */
> +       if (ret == -ERESTARTSYS)
> +               return ret;
> +
>         if (copy_to_user(arg, &range, sizeof(range)))
>                 return -EFAULT;
>
>
> ---
> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
> change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13
>
> Best regards,
> --
> Zhiwei Liang <zhiwei.liang@zliang.me>
>
>

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Qu Wenruo]

在 2026/10/28 19:21, Luca Stefani 写道:
> On Wed, 28 Oct 2026 at 07:57, Zhiwei Liang via B4 Relay
> <devnull+zhiwei.liang.zliang.me@kernel.org> wrote:
>>
>> From: Zhiwei Liang <zhiwei.liang@zliang.me>
>>
>> When a suspend starts during FITRIM, fstrim can fail with:
>>
>>    fstrim: /: FITRIM ioctl failed: Unknown error 512
>>
>> On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
>> fstrim.service runs were in progress when an automatic suspend started.
>> All three failed this way and left the filesystem untrimmed.
>>
>> Since commit 69313850dce3 ("btrfs: add cancellation points to trim
>> loops"), the trim loops return -ERESTARTSYS when
>> btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
>> to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
>> properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.
>>
>> -ERESTARTSYS is only valid while a signal is pending, because only the
>> signal handling on syscall exit turns it into a restart. But
>> freeze_processes() first sets pm_freezing, which makes freezing() true
>> for every user task. Then it walks the task list and sends each task a
>> fake signal. The trim loops check btrfs_trim_interrupted() after every
>> extent. If a check falls in this window, the loop returns -ERESTARTSYS
>> with no signal pending. Nothing restarts the syscall. On that laptop,
>> with a few thousand threads, freezing user space took 2 to 5 ms in each
>> of the three suspends above, and all three runs hit this window.
>>
>> Make btrfs_trim_interrupted() treat a freeze as an interruption only
>> once the fake signal has arrived, i.e. when
>> freezing(current) && signal_pending(current). freeze_processes() sends
>> that signal in the same call. The trim loop stops at the next check
>> after it, so suspend is not delayed.
> Looking at how ext4 handles this scenario it seems to me they don't set
> the error code to -ERESTARTSYS and (from a quick glance) return the
> bytes discarded to the caller.
> And like before they continue the discard (which will skip/fail again) until
> there's nothing to be done.
> 
> I don't know what the intention of btrfs: handle user interrupt
> properly in btrfs_trim_fs()
> was, maybe we should just swallow the ERESTARTSYS case in those scenarios.

I agree on that btrfs is the exception, we should follow ext4/xfs, not 
rely on some tricky signal handling behavior for suspension.

I think the tokens of the author should be spent on unifying the btrfs 
fstrim suspension behavior instead.


>>
>> The restarted syscall re-reads struct fstrim_range from user memory.
>> When the device phase is interrupted, btrfs_trim_fs() has already stored
>> the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
>> the struct back unconditionally. The restart is then asked to trim
>> [start, start + trimmed) instead of the requested range, or fails with
>> -EINVAL when nothing was counted yet. Skip the copy_to_user() for
>> -ERESTARTSYS. In all other cases the partial count is still copied back.
>>
>> Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
>> during a trim leaked errno 512, and 10 of 10 restarts after a
>> device-phase interruption failed with -EINVAL. With the patch, all 30
>> runs restarted and trimmed the full amount.
>>
>> Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
>> CC: stable@vger.kernel.org # 5.15+
>> Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
>> ---
>>   fs/btrfs/free-space-cache.h | 3 ++-
>>   fs/btrfs/ioctl.c            | 7 +++++++
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
>> index e22443598b..cba2f5fd6b 100644
>> --- a/fs/btrfs/free-space-cache.h
>> +++ b/fs/btrfs/free-space-cache.h
>> @@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
>>
>>   static inline bool btrfs_trim_interrupted(void)
>>   {
>> -       return fatal_signal_pending(current) || freezing(current);
>> +       return fatal_signal_pending(current) ||
>> +              (freezing(current) && signal_pending(current));
>>   }
>>
>>   /*
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index ac070c745b..b02c59cbb0 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
>>          range.minlen = max(range.minlen, minlen);
>>          ret = btrfs_trim_fs(fs_info, &range);
>>
>> +       /*
>> +        * A restart re-reads the range from user space, so do not clobber it
>> +        * with the partial len the rerun would otherwise take as its input.
>> +        */
>> +       if (ret == -ERESTARTSYS)
>> +               return ret;
>> +
>>          if (copy_to_user(arg, &range, sizeof(range)))
>>                  return -EFAULT;
>>
>>
>> ---
>> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
>> change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13
>>
>> Best regards,
>> --
>> Zhiwei Liang <zhiwei.liang@zliang.me>
>>
>>

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Luca Stefani]

On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@suse.com> wrote:
>
>
>
> 在 2026/10/28 19:21, Luca Stefani 写道:
> > On Wed, 28 Oct 2026 at 07:57, Zhiwei Liang via B4 Relay
> > <devnull+zhiwei.liang.zliang.me@kernel.org> wrote:
> >>
> >> From: Zhiwei Liang <zhiwei.liang@zliang.me>
> >>
> >> When a suspend starts during FITRIM, fstrim can fail with:
> >>
> >>    fstrim: /: FITRIM ioctl failed: Unknown error 512
> >>
> >> On a Fedora 44 laptop with kernel 7.2.7, 3 of the last 21 weekly
> >> fstrim.service runs were in progress when an automatic suspend started.
> >> All three failed this way and left the filesystem untrimmed.
> >>
> >> Since commit 69313850dce3 ("btrfs: add cancellation points to trim
> >> loops"), the trim loops return -ERESTARTSYS when
> >> btrfs_trim_interrupted() sees freezing(current), and FITRIM returns it
> >> to userspace. Since commit bfb670b9183b ("btrfs: handle user interrupt
> >> properly in btrfs_trim_fs()"), btrfs_trim_fs() returns it right away.
> >>
> >> -ERESTARTSYS is only valid while a signal is pending, because only the
> >> signal handling on syscall exit turns it into a restart. But
> >> freeze_processes() first sets pm_freezing, which makes freezing() true
> >> for every user task. Then it walks the task list and sends each task a
> >> fake signal. The trim loops check btrfs_trim_interrupted() after every
> >> extent. If a check falls in this window, the loop returns -ERESTARTSYS
> >> with no signal pending. Nothing restarts the syscall. On that laptop,
> >> with a few thousand threads, freezing user space took 2 to 5 ms in each
> >> of the three suspends above, and all three runs hit this window.
> >>
> >> Make btrfs_trim_interrupted() treat a freeze as an interruption only
> >> once the fake signal has arrived, i.e. when
> >> freezing(current) && signal_pending(current). freeze_processes() sends
> >> that signal in the same call. The trim loop stops at the next check
> >> after it, so suspend is not delayed.
> > Looking at how ext4 handles this scenario it seems to me they don't set
> > the error code to -ERESTARTSYS and (from a quick glance) return the
> > bytes discarded to the caller.
> > And like before they continue the discard (which will skip/fail again) until
> > there's nothing to be done.
> >
> > I don't know what the intention of btrfs: handle user interrupt
> > properly in btrfs_trim_fs()
> > was, maybe we should just swallow the ERESTARTSYS case in those scenarios.
>
> I agree on that btrfs is the exception, we should follow ext4/xfs, not
> rely on some tricky signal handling behavior for suspension.
When I updated btrfs_trim_interrupted I used the same "checks" as
seen in ext4 so at least on that part it's identical.

What differs is our error handling: we set/handle -ERESTARTSYS, ext4 does not.
ext4_try_to_trim_range only ever succeeds by returning the count of
trimmed bytes
even if it checks for trim interruptions.

I think if we remove ret = -ERESTARTSYS; from the
btrfs_trim_interrupted branches
we'd align the behavior (plus reverting the previous change as that's
now useless)
>
> I think the tokens of the author should be spent on unifying the btrfs
> fstrim suspension behavior instead.
>
>
> >>
> >> The restarted syscall re-reads struct fstrim_range from user memory.
> >> When the device phase is interrupted, btrfs_trim_fs() has already stored
> >> the trimmed byte count in range->len, and btrfs_ioctl_fitrim() copies
> >> the struct back unconditionally. The restart is then asked to trim
> >> [start, start + trimmed) instead of the requested range, or fails with
> >> -EINVAL when nothing was counted yet. Skip the copy_to_user() for
> >> -ERESTARTSYS. In all other cases the partial count is still copied back.
> >>
> >> Tested in a VM with pm_test=freezer. Without the patch, 18 of 20 freezes
> >> during a trim leaked errno 512, and 10 of 10 restarts after a
> >> device-phase interruption failed with -EINVAL. With the patch, all 30
> >> runs restarted and trimmed the full amount.
> >>
> >> Fixes: 69313850dce3 ("btrfs: add cancellation points to trim loops")
> >> CC: stable@vger.kernel.org # 5.15+
> >> Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
> >> ---
> >>   fs/btrfs/free-space-cache.h | 3 ++-
> >>   fs/btrfs/ioctl.c            | 7 +++++++
> >>   2 files changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/btrfs/free-space-cache.h b/fs/btrfs/free-space-cache.h
> >> index e22443598b..cba2f5fd6b 100644
> >> --- a/fs/btrfs/free-space-cache.h
> >> +++ b/fs/btrfs/free-space-cache.h
> >> @@ -58,7 +58,8 @@ static inline bool btrfs_free_space_trimming_bitmap(
> >>
> >>   static inline bool btrfs_trim_interrupted(void)
> >>   {
> >> -       return fatal_signal_pending(current) || freezing(current);
> >> +       return fatal_signal_pending(current) ||
> >> +              (freezing(current) && signal_pending(current));
> >>   }
> >>
> >>   /*
> >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> >> index ac070c745b..b02c59cbb0 100644
> >> --- a/fs/btrfs/ioctl.c
> >> +++ b/fs/btrfs/ioctl.c
> >> @@ -529,6 +529,13 @@ static noinline int btrfs_ioctl_fitrim(struct btrfs_fs_info *fs_info,
> >>          range.minlen = max(range.minlen, minlen);
> >>          ret = btrfs_trim_fs(fs_info, &range);
> >>
> >> +       /*
> >> +        * A restart re-reads the range from user space, so do not clobber it
> >> +        * with the partial len the rerun would otherwise take as its input.
> >> +        */
> >> +       if (ret == -ERESTARTSYS)
> >> +               return ret;
> >> +
> >>          if (copy_to_user(arg, &range, sizeof(range)))
> >>                  return -EFAULT;
> >>
> >>
> >> ---
> >> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
> >> change-id: 20261028-btrfs-fitrim-freezer-d4cba16e7e13
> >>
> >> Best regards,
> >> --
> >> Zhiwei Liang <zhiwei.liang@zliang.me>
> >>
> >>
>

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Qu Wenruo]

在 2026/10/28 19:50, Luca Stefani 写道:
> On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@suse.com> wrote:
[...]
>>> I don't know what the intention of btrfs: handle user interrupt
>>> properly in btrfs_trim_fs()
>>> was, maybe we should just swallow the ERESTARTSYS case in those scenarios.
>>
>> I agree on that btrfs is the exception, we should follow ext4/xfs, not
>> rely on some tricky signal handling behavior for suspension.
> When I updated btrfs_trim_interrupted I used the same "checks" as
> seen in ext4 so at least on that part it's identical.
> 
> What differs is our error handling: we set/handle -ERESTARTSYS, ext4 does not.
> ext4_try_to_trim_range only ever succeeds by returning the count of
> trimmed bytes
> even if it checks for trim interruptions.
> 
> I think if we remove ret = -ERESTARTSYS; from the
> btrfs_trim_interrupted branches
> we'd align the behavior (plus reverting the previous change as that's
> now useless)

I think that's would work for most call sites, but there are some call 
sites that we may want to know if it's interrupted.

E.g. btrfs_issue_discard() is very deep inside all trim calls, if we 
drop the return value without any other extra way to notify the caller, 
it will incorrectly return 0, which is definitely not a good idea.

A safer solution would be keep the existing ERESTARTSYS, but change it 
to return 0 for now.

But if you have a better solution and can completely remove ERESTARTSYS, 
I can't be more happier.

Thanks,
Qu

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Qu Wenruo]

在 2026/10/28 20:12, Qu Wenruo 写道:
> 
> 
> 在 2026/10/28 19:50, Luca Stefani 写道:
>> On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@suse.com> wrote:
> [...]
>>>> I don't know what the intention of btrfs: handle user interrupt
>>>> properly in btrfs_trim_fs()
>>>> was, maybe we should just swallow the ERESTARTSYS case in those 
>>>> scenarios.
>>>
>>> I agree on that btrfs is the exception, we should follow ext4/xfs, not
>>> rely on some tricky signal handling behavior for suspension.
>> When I updated btrfs_trim_interrupted I used the same "checks" as
>> seen in ext4 so at least on that part it's identical.
>>
>> What differs is our error handling: we set/handle -ERESTARTSYS, ext4 
>> does not.
>> ext4_try_to_trim_range only ever succeeds by returning the count of
>> trimmed bytes
>> even if it checks for trim interruptions.
>>
>> I think if we remove ret = -ERESTARTSYS; from the
>> btrfs_trim_interrupted branches
>> we'd align the behavior (plus reverting the previous change as that's
>> now useless)
> 
> I think that's would work for most call sites, but there are some call 
> sites that we may want to know if it's interrupted.
> 
> E.g. btrfs_issue_discard() is very deep inside all trim calls, if we 
> drop the return value without any other extra way to notify the caller, 
> it will incorrectly return 0, which is definitely not a good idea.
> 
> A safer solution would be keep the existing ERESTARTSYS, but change it 
> to return 0 for now.

Sorry, I mean to return 0 at the fstrim entrance, btrfs_trim_fs().

> 
> But if you have a better solution and can completely remove ERESTARTSYS, 
> I can't be more happier.
> 
> Thanks,
> Qu

Re: [PATCH] btrfs: do not leak -ERESTARTSYS from FITRIM to userspace

[Luca Stefani]

On Wed, 28 Oct 2026 at 10:42, Qu Wenruo <wqu@suse.com> wrote:
>
>
>
> 在 2026/10/28 19:50, Luca Stefani 写道:
> > On Wed, 28 Oct 2026 at 09:57, Qu Wenruo <wqu@suse.com> wrote:
> [...]
> >>> I don't know what the intention of btrfs: handle user interrupt
> >>> properly in btrfs_trim_fs()
> >>> was, maybe we should just swallow the ERESTARTSYS case in those scenarios.
> >>
> >> I agree on that btrfs is the exception, we should follow ext4/xfs, not
> >> rely on some tricky signal handling behavior for suspension.
> > When I updated btrfs_trim_interrupted I used the same "checks" as
> > seen in ext4 so at least on that part it's identical.
> >
> > What differs is our error handling: we set/handle -ERESTARTSYS, ext4 does not.
> > ext4_try_to_trim_range only ever succeeds by returning the count of
> > trimmed bytes
> > even if it checks for trim interruptions.
> >
> > I think if we remove ret = -ERESTARTSYS; from the
> > btrfs_trim_interrupted branches
> > we'd align the behavior (plus reverting the previous change as that's
> > now useless)
>
> I think that's would work for most call sites, but there are some call
> sites that we may want to know if it's interrupted.
>
> E.g. btrfs_issue_discard() is very deep inside all trim calls, if we
> drop the return value without any other extra way to notify the caller,
> it will incorrectly return 0, which is definitely not a good idea.
>
> A safer solution would be keep the existing ERESTARTSYS, but change it
> to return 0 for now.
That would work, I think.
But that would only solve the userspace problem, as checking for
freezing(current) might be wrong in those points, which was pointed
out in this change.
Maybe the "old" btrfs_trim_interrupted users should go back to
fatal_signal_pending(current) only while the free discarding phase
keep it.

This looks similar to what some places in ext4 do, but I'm no expert.
>
> But if you have a better solution and can completely remove ERESTARTSYS,
> I can't be more happier.
I wish I could help, but as long as it's a targeted change I'd be
happy to help, but I don't know enough about the fs :)
>
> Thanks,
> Qu