diff mbox series

[v2] vfs: generic_copy_file_checks should return EINVAL when source offset is beyond EOF

Message ID 20210201204952.74625-1-dai.ngo@oracle.com (mailing list archive)
State New
Headers show
Series [v2] vfs: generic_copy_file_checks should return EINVAL when source offset is beyond EOF | expand

Commit Message

Dai Ngo Feb. 1, 2021, 8:49 p.m. UTC
Fix by returning -EINVAL instead of 0, per man page of copy_file_range,
when the requested range extends beyond the end of the source file.
Problem was discovered by subtest inter11 of nfstest_ssc.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/read_write.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Darrick J. Wong Feb. 1, 2021, 9:14 p.m. UTC | #1
On Mon, Feb 01, 2021 at 03:49:52PM -0500, Dai Ngo wrote:
> Fix by returning -EINVAL instead of 0, per man page of copy_file_range,

Huh?  That's not what the manpage[1] says:

RETURN VALUE
       Upon successful completion, copy_file_range() will return the
number of bytes copied between files.  This could be less than the
length originally requested.  If the file offset of fd_in is at or past
the end of file, no bytes are copied, and copy_file_range() returns
zero.

--D

[1] https://man7.org/linux/man-pages/man2/copy_file_range.2.html#RETURN_VALUE

> when the requested range extends beyond the end of the source file.
> Problem was discovered by subtest inter11 of nfstest_ssc.
> 
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/read_write.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 75f764b43418..438c00910716 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -1445,7 +1445,7 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
>  	/* Shorten the copy to EOF */
>  	size_in = i_size_read(inode_in);
>  	if (pos_in >= size_in)
> -		count = 0;
> +		count = -EINVAL;
>  	else
>  		count = min(count, size_in - (uint64_t)pos_in);
>  
> -- 
> 2.9.5
>
Dai Ngo Feb. 1, 2021, 9:51 p.m. UTC | #2
On 2/1/21 1:14 PM, Darrick J. Wong wrote:
> On Mon, Feb 01, 2021 at 03:49:52PM -0500, Dai Ngo wrote:
>> Fix by returning -EINVAL instead of 0, per man page of copy_file_range,
> Huh?  That's not what the manpage[1] says:
>
> RETURN VALUE
>         Upon successful completion, copy_file_range() will return the
> number of bytes copied between files.  This could be less than the
> length originally requested.  If the file offset of fd_in is at or past
> the end of file, no bytes are copied, and copy_file_range() returns
> zero.

In the ERROR section:

ERROR
	EINVAL Requested range extends beyond the end of the source file; or the flags argument is not 0.

-Dai

>
> --D
>
> [1] https://urldefense.com/v3/__https://man7.org/linux/man-pages/man2/copy_file_range.2.html*RETURN_VALUE__;Iw!!GqivPVa7Brio!N8XGLyK9dUTWOYWmb3NCuq-9kL-tX8OGcq-sV7M53ub5KMgr7e93a2B8eUryKw$
>
>> when the requested range extends beyond the end of the source file.
>> Problem was discovered by subtest inter11 of nfstest_ssc.
>>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>>   fs/read_write.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/read_write.c b/fs/read_write.c
>> index 75f764b43418..438c00910716 100644
>> --- a/fs/read_write.c
>> +++ b/fs/read_write.c
>> @@ -1445,7 +1445,7 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
>>   	/* Shorten the copy to EOF */
>>   	size_in = i_size_read(inode_in);
>>   	if (pos_in >= size_in)
>> -		count = 0;
>> +		count = -EINVAL;
>>   	else
>>   		count = min(count, size_in - (uint64_t)pos_in);
>>   
>> -- 
>> 2.9.5
>>
Dai Ngo Feb. 11, 2021, 7:54 p.m. UTC | #3
Hi Darrick,

My man page was out-of-date. The latest man page of coy_file_range is more clear:

This EINVAL was removed:

        EINVAL Requested range extends beyond the end of the source file; or the flags argument is not 0.

and replaced with this:
         
*EINVAL *The/flags/  argument is not 0

Thanks,
-Dai

On 2/1/21 1:51 PM, dai.ngo@oracle.com wrote:
>
> On 2/1/21 1:14 PM, Darrick J. Wong wrote:
>> On Mon, Feb 01, 2021 at 03:49:52PM -0500, Dai Ngo wrote:
>>> Fix by returning -EINVAL instead of 0, per man page of copy_file_range,
>> Huh?  That's not what the manpage[1] says:
>>
>> RETURN VALUE
>>         Upon successful completion, copy_file_range() will return the
>> number of bytes copied between files.  This could be less than the
>> length originally requested.  If the file offset of fd_in is at or past
>> the end of file, no bytes are copied, and copy_file_range() returns
>> zero.
>
> In the ERROR section:
>
> ERROR
>     EINVAL Requested range extends beyond the end of the source file; 
> or the flags argument is not 0.
>
> -Dai
>
>>
>> --D
>>
>> [1] 
>> https://urldefense.com/v3/__https://man7.org/linux/man-pages/man2/copy_file_range.2.html*RETURN_VALUE__;Iw!!GqivPVa7Brio!N8XGLyK9dUTWOYWmb3NCuq-9kL-tX8OGcq-sV7M53ub5KMgr7e93a2B8eUryKw$
>>
>>> when the requested range extends beyond the end of the source file.
>>> Problem was discovered by subtest inter11 of nfstest_ssc.
>>>
>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>>> ---
>>>   fs/read_write.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/read_write.c b/fs/read_write.c
>>> index 75f764b43418..438c00910716 100644
>>> --- a/fs/read_write.c
>>> +++ b/fs/read_write.c
>>> @@ -1445,7 +1445,7 @@ static int generic_copy_file_checks(struct 
>>> file *file_in, loff_t pos_in,
>>>       /* Shorten the copy to EOF */
>>>       size_in = i_size_read(inode_in);
>>>       if (pos_in >= size_in)
>>> -        count = 0;
>>> +        count = -EINVAL;
>>>       else
>>>           count = min(count, size_in - (uint64_t)pos_in);
>>>   --
>>> 2.9.5
>>>
diff mbox series

Patch

diff --git a/fs/read_write.c b/fs/read_write.c
index 75f764b43418..438c00910716 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1445,7 +1445,7 @@  static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
 	/* Shorten the copy to EOF */
 	size_in = i_size_read(inode_in);
 	if (pos_in >= size_in)
-		count = 0;
+		count = -EINVAL;
 	else
 		count = min(count, size_in - (uint64_t)pos_in);