Hi Yilu, I think your reasoning makes sense. Do you have a small reproducer for this? A small C program that triggers this? I am asking because if you do we would like to add it to our buildbot to make sure we don't get regressions. regards ronnie sahlberg On Wed, Mar 18, 2020 at 1:59 PM Yilu Lin <linyilu@xxxxxxxxxx> wrote: > > This patch is used to fix the bug in collect_uncached_read_data() > that rc is automatically converted from a signed number to an > unsigned number when the CIFS asynchronous read fails. > It will cause ctx->rc is error. > > Example: > Share a directory and create a file on the Windows OS. > Mount the directory to the Linux OS using CIFS. > On the CIFS client of the Linux OS, invoke the pread interface to > deliver the read request. > > The size of the read length plus offset of the read request is greater > than the maximum file size. > > In this case, the CIFS server on the Windows OS returns a failure > message (for example, the return value of > smb2.nt_status is STATUS_INVALID_PARAMETER). > > After receiving the response message, the CIFS client parses > smb2.nt_status to STATUS_INVALID_PARAMETER > and converts it to the Linux error code (rdata->result=-22). > > Then the CIFS client invokes the collect_uncached_read_data function to > assign the value of rdata->result to rc, that is, rc=rdata->result=-22. > > The type of the ctx->total_len variable is unsigned integer, > the type of the rc variable is integer, and the type of > the ctx->rc variable is ssize_t. > > Therefore, during the ternary operation, the value of rc is > automatically converted to an unsigned number. The final result is > ctx->rc=4294967274. However, the expected result is ctx->rc=-22. > > Signed-off-by: Yilu Lin <linyilu@xxxxxxxxxx> > --- > fs/cifs/file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index 022029a5d..ff4ac244c 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -3323,7 +3323,7 @@ again: > if (rc == -ENODATA) > rc = 0; > > - ctx->rc = (rc == 0) ? ctx->total_len : rc; > + ctx->rc = (rc == 0) ? (ssize_t)ctx->total_len : rc; > > mutex_unlock(&ctx->aio_mutex); > > -- > 2.19.1 > >
