Re: [PATCH] netfs: Fix unbuffered write error handling

[Paulo Alcantara]

David Howells <dhowells@xxxxxxxxxx> 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@xxxxxxxxx>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220445
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> cc: Paulo Alcantara <pc@xxxxxxxxxxxxx>
> cc: Steve French <sfrench@xxxxxxxxx>
> cc: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
> cc: netfs@xxxxxxxxxxxxxxx
> cc: linux-cifs@xxxxxxxxxxxxxxx
> cc: linux-fsdevel@xxxxxxxxxxxxxxx
> cc: stable@xxxxxxxxxxxxxxx
> ---
>  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@xxxxxxxxxxxxx>