[PATCH] nilfs2: ignore non-fatal signals during synchronous log write wait

[PATCH] nilfs2: ignore non-fatal signals during synchronous log write wait

[Ryusuke Konishi]

Based on a syzbot report, Wang Jianjian pointed out that following a
successful directory operation in nilfs_rename(), a synchronous write
requested within nilfs_transaction_commit(), triggered by a sync flag
set by nilfs_commit_chunk(), could be interrupted by a user signal.
This interruption leaves in-memory metadata updates intact while
returning an error, causing inconsistency with the VFS dentry cache and
ultimately triggering a kernel warning.

According to "signal(7)", local disks are not classified as "slow
devices" (such as terminals, FIFOs, or pipes) and their I/O operations
on disk devices are not interrupted by signals.  In kernel space,
waiting for such disk I/O should therefore not be aborted by ordinary
(non-fatal) signals.

Suppress this issue by modifying nilfs_segctor_sync(), which waits for
the log writer thread to complete synchronous writes, to catch only
fatal signals via fatal_signal_pending() and return -EINTR rather than
-ERESTARTSYS.  To complement this change and prevent the wait loop from
busy-looping when schedule() returns immediately due to a pending
non-fatal signal, switch the wait state from TASK_INTERRUPTIBLE to
TASK_KILLABLE.

This serves as an effective interim stabilization measure without
structural changes until a comprehensive fix for metadata consistency
is implemented.

Reported-by: syzbot+f6c7e1f1809f235eeb90@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f6c7e1f1809f235eeb90
Fixes: 9ff05123e3bf ("nilfs2: segment constructor")
Tested-by: syzbot+f6c7e1f1809f235eeb90@syzkaller.appspotmail.com
Cc: Wang Jianjian <wangjianjian3@huawei.com>
Cc: stable@vger.kernel.org
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
---
Hi Viacheslav,

Please queue this for the next cycle.

This is the first step toward addressing the state inconsistencies
during directory operations reported by syzbot or on the mailing lists.
It improves stability by preventing errors caused by user interrupts
while waiting for log writes, which are triggered by synchronous write
flags following operations such as rename().

As this does not fully address the underlying issues in error handling,
I intend to continue exploring changes that allow directory operations
to be properly canceled or rolled back.

Thanks,
Ryusuke Konishi

 fs/nilfs2/ioctl.c    |  2 +-
 fs/nilfs2/recovery.c |  2 +-
 fs/nilfs2/segment.c  | 10 +++++-----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index 01a04080ef70..b2b67d6a1f09 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -965,10 +965,10 @@ static int nilfs_ioctl_clean_segments(struct inode *inode, struct file *filp,
  * Return: 0 on success, or one of the following negative error codes on
  * failure:
  * * %-EFAULT		- Failure during execution of requested operation.
+ * * %-EINTR		- Interrupted.
  * * %-EIO		- I/O error.
  * * %-ENOMEM		- Insufficient memory available.
  * * %-ENOSPC		- No space left on device (only in a panic state).
- * * %-ERESTARTSYS	- Interrupted.
  * * %-EROFS		- Read only filesystem.
  */
 static int nilfs_ioctl_sync(struct inode *inode, struct file *filp,
diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
index 9a70c28ec94c..abe4101f7550 100644
--- a/fs/nilfs2/recovery.c
+++ b/fs/nilfs2/recovery.c
@@ -790,11 +790,11 @@ static void nilfs_abort_roll_forward(struct the_nilfs *nilfs)
  *
  * Return: 0 on success, or one of the following negative error codes on
  * failure:
+ * * %-EINTR		- Interrupted.
  * * %-EINVAL		- Inconsistent filesystem state.
  * * %-EIO		- I/O error.
  * * %-ENOMEM		- Insufficient memory available.
  * * %-ENOSPC		- No space left on device (only in a panic state).
- * * %-ERESTARTSYS	- Interrupted.
  */
 int nilfs_salvage_orphan_logs(struct the_nilfs *nilfs,
 			      struct super_block *sb,
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index 2f0e59847d02..af0c3560444f 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2259,7 +2259,7 @@ static int nilfs_segctor_sync(struct nilfs_sc_info *sci)
 	wake_up(&sci->sc_wait_daemon);
 
 	for (;;) {
-		set_current_state(TASK_INTERRUPTIBLE);
+		set_current_state(TASK_KILLABLE);
 
 		/*
 		 * Synchronize only while the log writer thread is alive.
@@ -2273,11 +2273,11 @@ static int nilfs_segctor_sync(struct nilfs_sc_info *sci)
 			err = wait_req.err;
 			break;
 		}
-		if (!signal_pending(current)) {
+		if (!fatal_signal_pending(current)) {
 			schedule();
 			continue;
 		}
-		err = -ERESTARTSYS;
+		err = -EINTR;
 		break;
 	}
 	finish_wait(&sci->sc_wait_request, &wait_req.wq);
@@ -2311,10 +2311,10 @@ static void nilfs_segctor_wakeup(struct nilfs_sc_info *sci, int err, bool force)
  *
  * Return: 0 on success, or one of the following negative error codes on
  * failure:
+ * * %-EINTR		- Interrupted.
  * * %-EIO		- I/O error (including metadata corruption).
  * * %-ENOMEM		- Insufficient memory available.
  * * %-ENOSPC		- No space left on device (only in a panic state).
- * * %-ERESTARTSYS	- Interrupted.
  * * %-EROFS		- Read only filesystem.
  */
 int nilfs_construct_segment(struct super_block *sb)
@@ -2341,10 +2341,10 @@ int nilfs_construct_segment(struct super_block *sb)
  *
  * Return: 0 on success, or one of the following negative error codes on
  * failure:
+ * * %-EINTR		- Interrupted.
  * * %-EIO		- I/O error (including metadata corruption).
  * * %-ENOMEM		- Insufficient memory available.
  * * %-ENOSPC		- No space left on device (only in a panic state).
- * * %-ERESTARTSYS	- Interrupted.
  * * %-EROFS		- Read only filesystem.
  */
 int nilfs_construct_dsync_segment(struct super_block *sb, struct inode *inode,
-- 
2.53.0

Re: [PATCH] nilfs2: ignore non-fatal signals during synchronous log write wait

[Viacheslav Dubeyko]

On Wed, 2026-09-30 at 03:26 +0900, Ryusuke Konishi wrote:
> Based on a syzbot report, Wang Jianjian pointed out that following a
> successful directory operation in nilfs_rename(), a synchronous write
> requested within nilfs_transaction_commit(), triggered by a sync flag
> set by nilfs_commit_chunk(), could be interrupted by a user signal.
> This interruption leaves in-memory metadata updates intact while
> returning an error, causing inconsistency with the VFS dentry cache
> and
> ultimately triggering a kernel warning.
> 
> According to "signal(7)", local disks are not classified as "slow
> devices" (such as terminals, FIFOs, or pipes) and their I/O
> operations
> on disk devices are not interrupted by signals.  In kernel space,
> waiting for such disk I/O should therefore not be aborted by ordinary
> (non-fatal) signals.
> 
> Suppress this issue by modifying nilfs_segctor_sync(), which waits
> for
> the log writer thread to complete synchronous writes, to catch only
> fatal signals via fatal_signal_pending() and return -EINTR rather
> than
> -ERESTARTSYS.  To complement this change and prevent the wait loop
> from
> busy-looping when schedule() returns immediately due to a pending
> non-fatal signal, switch the wait state from TASK_INTERRUPTIBLE to
> TASK_KILLABLE.
> 
> This serves as an effective interim stabilization measure without
> structural changes until a comprehensive fix for metadata consistency
> is implemented.
> 
> Reported-by: syzbot+f6c7e1f1809f235eeb90@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f6c7e1f1809f235eeb90
> Fixes: 9ff05123e3bf ("nilfs2: segment constructor")
> Tested-by: syzbot+f6c7e1f1809f235eeb90@syzkaller.appspotmail.com
> Cc: Wang Jianjian <wangjianjian3@huawei.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> ---
> Hi Viacheslav,
> 
> Please queue this for the next cycle.
> 
> This is the first step toward addressing the state inconsistencies
> during directory operations reported by syzbot or on the mailing
> lists.
> It improves stability by preventing errors caused by user interrupts
> while waiting for log writes, which are triggered by synchronous
> write
> flags following operations such as rename().
> 
> As this does not fully address the underlying issues in error
> handling,
> I intend to continue exploring changes that allow directory
> operations
> to be properly canceled or rolled back.
> 
> Thanks,
> Ryusuke Konishi
> 
>  fs/nilfs2/ioctl.c    |  2 +-
>  fs/nilfs2/recovery.c |  2 +-
>  fs/nilfs2/segment.c  | 10 +++++-----
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> index 01a04080ef70..b2b67d6a1f09 100644
> --- a/fs/nilfs2/ioctl.c
> +++ b/fs/nilfs2/ioctl.c
> @@ -965,10 +965,10 @@ static int nilfs_ioctl_clean_segments(struct
> inode *inode, struct file *filp,
>   * Return: 0 on success, or one of the following negative error
> codes on
>   * failure:
>   * * %-EFAULT		- Failure during execution of requested
> operation.
> + * * %-EINTR		- Interrupted.
>   * * %-EIO		- I/O error.
>   * * %-ENOMEM		- Insufficient memory available.
>   * * %-ENOSPC		- No space left on device (only in a panic
> state).
> - * * %-ERESTARTSYS	- Interrupted.
>   * * %-EROFS		- Read only filesystem.
>   */
>  static int nilfs_ioctl_sync(struct inode *inode, struct file *filp,
> diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
> index 9a70c28ec94c..abe4101f7550 100644
> --- a/fs/nilfs2/recovery.c
> +++ b/fs/nilfs2/recovery.c
> @@ -790,11 +790,11 @@ static void nilfs_abort_roll_forward(struct
> the_nilfs *nilfs)
>   *
>   * Return: 0 on success, or one of the following negative error
> codes on
>   * failure:
> + * * %-EINTR		- Interrupted.
>   * * %-EINVAL		- Inconsistent filesystem state.
>   * * %-EIO		- I/O error.
>   * * %-ENOMEM		- Insufficient memory available.
>   * * %-ENOSPC		- No space left on device (only in a panic
> state).
> - * * %-ERESTARTSYS	- Interrupted.
>   */
>  int nilfs_salvage_orphan_logs(struct the_nilfs *nilfs,
>  			      struct super_block *sb,
> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> index 2f0e59847d02..af0c3560444f 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -2259,7 +2259,7 @@ static int nilfs_segctor_sync(struct
> nilfs_sc_info *sci)
>  	wake_up(&sci->sc_wait_daemon);
>  
>  	for (;;) {
> -		set_current_state(TASK_INTERRUPTIBLE);
> +		set_current_state(TASK_KILLABLE);
>  
>  		/*
>  		 * Synchronize only while the log writer thread is
> alive.
> @@ -2273,11 +2273,11 @@ static int nilfs_segctor_sync(struct
> nilfs_sc_info *sci)
>  			err = wait_req.err;
>  			break;
>  		}
> -		if (!signal_pending(current)) {
> +		if (!fatal_signal_pending(current)) {
>  			schedule();
>  			continue;
>  		}
> -		err = -ERESTARTSYS;
> +		err = -EINTR;
>  		break;
>  	}
>  	finish_wait(&sci->sc_wait_request, &wait_req.wq);
> @@ -2311,10 +2311,10 @@ static void nilfs_segctor_wakeup(struct
> nilfs_sc_info *sci, int err, bool force)
>   *
>   * Return: 0 on success, or one of the following negative error
> codes on
>   * failure:
> + * * %-EINTR		- Interrupted.
>   * * %-EIO		- I/O error (including metadata corruption).
>   * * %-ENOMEM		- Insufficient memory available.
>   * * %-ENOSPC		- No space left on device (only in a panic
> state).
> - * * %-ERESTARTSYS	- Interrupted.
>   * * %-EROFS		- Read only filesystem.
>   */
>  int nilfs_construct_segment(struct super_block *sb)
> @@ -2341,10 +2341,10 @@ int nilfs_construct_segment(struct
> super_block *sb)
>   *
>   * Return: 0 on success, or one of the following negative error
> codes on
>   * failure:
> + * * %-EINTR		- Interrupted.
>   * * %-EIO		- I/O error (including metadata corruption).
>   * * %-ENOMEM		- Insufficient memory available.
>   * * %-ENOSPC		- No space left on device (only in a panic
> state).
> - * * %-ERESTARTSYS	- Interrupted.
>   * * %-EROFS		- Read only filesystem.
>   */
>  int nilfs_construct_dsync_segment(struct super_block *sb, struct
> inode *inode,


Applied.

Thanks,
Slava.