Re: Data Corruption bug with Samba's vfs_iouring and Linux 5.6.7/5.7rc3

Jeremy Allison
2020-05-06T18:01:47Z

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

On Wed, May 06, 2020 at 10:03:44AM -0700, Jeremy Allison via samba-technical wrote:
> On Wed, May 06, 2020 at 04:08:03PM +0200, Stefan Metzmacher wrote:
> >
> > smbclient is just smart enough to recover itself from the short read.
> > But the windows client isn't.
>
> Well we pay attention to the amount of data returned
> and only increment the next read request by the amount
> actually returned.
>
> I'm amazed that the Windows client doesn't seem to
> check this !

Confirmed. I just ran a copy test from a Windows10
client copying from Samba with the attached patch
applied (when reading from offset zero, reduce
the amount of data returned by 2 bytes to force
a short read return), and it reliably corrupts files.

Windows isn't looking at the DataLength field
of the SMB2_READ response :-(.

[-- Attachment #2: look --]
[-- Type: text/plain, Size: 1015 bytes --]

diff --git a/source3/modules/vfs_io_uring.c b/source3/modules/vfs_io_uring.c
index 378e48d112f..d21a3485536 100644
--- a/source3/modules/vfs_io_uring.c
+++ b/source3/modules/vfs_io_uring.c
@@ -294,6 +294,7 @@ static void vfs_io_uring_fd_handler(struct tevent_context *ev,
struct vfs_io_uring_pread_state {
struct vfs_io_uring_request ur;
struct iovec iov;
+ off_t offset;
};

static struct tevent_req *vfs_io_uring_pread_send(struct vfs_handle_struct *handle,
@@ -319,6 +320,7 @@ static struct tevent_req *vfs_io_uring_pread_send(struct vfs_handle_struct *hand
state->ur.config = config;
state->ur.req = req;
state->ur.state = state;
+ state->offset = offset;

SMBPROFILE_BYTES_ASYNC_START(syscall_asys_pread, profile_p,
state->ur.profile_bytes, n);
@@ -367,6 +369,11 @@ static ssize_t vfs_io_uring_pread_recv(struct tevent_req *req,
ret = state->ur.cqe.res;
}

+ //JRATEST
+ if (ret > 2 && state->offset == 0) {
+ ret = ret - 2;
+ }
+
tevent_req_received(req);
return ret;
}