linux-kernel - Re: [PATCH] netfs: Fix unbuffered write error handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6ae175d2126c31b023ef1bc6d61a0a39@manguebit.org>
Date: Fri, 15 Aug 2025 13:09:35 -0300
From: Paulo Alcantara <pc@...guebit.org>
To: David Howells <dhowells@...hat.com>, Christian Brauner
 <brauner@...nel.org>, Steve French <sfrench@...ba.org>
Cc: dhowells@...hat.com, Xiaoli Feng <fengxiaoli0714@...il.com>, Shyam
 Prasad N <sprasad@...rosoft.com>, netfs@...ts.linux.dev,
 linux-cifs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 stable@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] netfs: Fix unbuffered write error handling

David Howells <dhowells@...hat.com> writes:

> If all the subrequests in an unbuffered write stream fail, the subrequest
> collector doesn't update the stream->transferred value and it retains its
> initial LONG_MAX value.  Unfortunately, if all active streams fail, then we
> take the smallest value of { LONG_MAX, LONG_MAX, ... } as the value to set
> in wreq->transferred - which is then returned from ->write_iter().
>
> LONG_MAX was chosen as the initial value so that all the streams can be
> quickly assessed by taking the smallest value of all stream->transferred -
> but this only works if we've set any of them.
>
> Fix this by adding a flag to indicate whether the value in
> stream->transferred is valid and checking that when we integrate the
> values.  stream->transferred can then be initialised to zero.
>
> This was found by running the generic/750 xfstest against cifs with
> cache=none.  It splices data to the target file.  Once (if) it has used up
> all the available scratch space, the writes start failing with ENOSPC.
> This causes ->write_iter() to fail.  However, it was returning
> wreq->transferred, i.e. LONG_MAX, rather than an error (because it thought
> the amount transferred was non-zero) and iter_file_splice_write() would
> then try to clean up that amount of pipe bufferage - leading to an oops
> when it overran.  The kernel log showed:
>
>     CIFS: VFS: Send error in write = -28
>
> followed by:
>
>     BUG: kernel NULL pointer dereference, address: 0000000000000008
>
> with:
>
>     RIP: 0010:iter_file_splice_write+0x3a4/0x520
>     do_splice+0x197/0x4e0
>
> or:
>
>     RIP: 0010:pipe_buf_release (include/linux/pipe_fs_i.h:282)
>     iter_file_splice_write (fs/splice.c:755)
>
> Also put a warning check into splice to announce if ->write_iter() returned
> that it had written more than it was asked to.
>
> Fixes: 288ace2f57c9 ("netfs: New writeback implementation")
> Reported-by: Xiaoli Feng <fengxiaoli0714@...il.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220445
> Signed-off-by: David Howells <dhowells@...hat.com>
> cc: Paulo Alcantara <pc@...guebit.org>
> cc: Steve French <sfrench@...ba.org>
> cc: Shyam Prasad N <sprasad@...rosoft.com>
> cc: netfs@...ts.linux.dev
> cc: linux-cifs@...r.kernel.org
> cc: linux-fsdevel@...r.kernel.org
> cc: stable@...r.kernel.org
> ---
>  fs/netfs/read_collect.c  |    4 +++-
>  fs/netfs/write_collect.c |   10 ++++++++--
>  fs/netfs/write_issue.c   |    4 ++--
>  fs/splice.c              |    3 +++
>  include/linux/netfs.h    |    1 +
>  5 files changed, 17 insertions(+), 5 deletions(-)

Reviewed-by: Paulo Alcantara (Red Hat) <pc@...guebit.org>