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

[Viacheslav Dubeyko <slava@dubeyko.com>]

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.