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@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
>>
>>
>
> .
>
