[PATCH] CIFS: Fix bug which the return value by asynchronous read is error

[Steve French]

Merged into cifs-2.6.git for-next

On Wed, Mar 18, 2020, 21:50 Yilu Lin <linyilu at huawei.com> wrote:

> Hi,
> The bug is trigerred by the following test program.
> First, run the command to create one file on the share
> directory。COMMAND：dd if=/dev/zero of=/home/temp/cifs/sys.zero bs=512
> count=1000 oflag=direct
> And then run the c program by the command: ./pread /home/temp/cifs/sys.zero
> The program will return "pread -22 bytes".However, the expected result is
> "pread -1 bytes".
>
> C program code is showed below:
>
>     #include<stdio.h>
>     #include<stdlib.h>
>     #include<unistd.h>
>     #include <sys/types.h>
>     #include <sys/stat.h>
>     #define __USE_GNU 1
>     #include <fcntl.h>
>     int main(int argc, char *argv[])
>     {
>             int fd;
>             ssize_t len;
>             int ret;
>             int size = 512;
>             char *file=argv[1];
>
>             unsigned char *buf2;
>             ret = posix_memalign((void **)&buf2, 512, size);
>             if (ret) {
>                     printf("malloc err!\n");
>                     return 0;
>             }
>
>             fd = open(file, O_RDWR | O_DIRECT);
>             if(fd == -1) {
>                     printf("open error!\n");
>                     free(buf2);
>                     return 0;
>             }
>
>             len = pread(fd, buf2, 504, 511992);
>             printf("pread %d bytes\n", len);
>             free(buf2);
>             close(fd);
>             return 0;
>     }
>
> regards
> Yilu Lin
>
> 在 2020/3/18 12:47, ronnie sahlberg 写道:
> > 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 at huawei.com> 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 at huawei.com>
> >> ---
> >>  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
> >>
> >>
> >
> > .
> >
>
>