On 2019/8/19 9:33, Chao Yu wrote:
> On 2019/8/18 23:41, Eric Biggers wrote:
>> On Fri, Aug 16, 2019 at 02:59:37PM +0800, Chao Yu wrote:
>>> On 2019/8/16 13:55, Eric Biggers wrote:
>>>> From: Eric Biggers <[email protected]>
>>>>
>>>> Userspace provides a null-terminated string, so don't assume that the
>>>> full FSLABEL_MAX bytes can always be copied.>
>>>> Fixes: 61a3da4d5ef8 ("f2fs: support FS_IOC_{GET,SET}FSLABEL")
>>>
>>> It may only copy redundant zero bytes, and will not hit security issue, it
>>> doesn't look like a bug fix?
>>>
>>>> Signed-off-by: Eric Biggers <[email protected]>
>>>
>>> Anyway, it makes sense to me.
>>>
>>> Reviewed-by: Chao Yu <[email protected]>
>>>
>>
>> It's not clear that userspace is guaranteed to provide a full FSLABEL_MAX
>> bytes
>> in the buffer. E.g. it could provide "foo\0" followed by an unmapped page.
>
> You're right, thanks for your explanation.
One more question, there is no validation check on length of user passed buffer,
So in most ioctl interfaces, user can pass a buffer which has less size than we
defined intentionally/unintentionally.
E.g.
user space:
struct f2fs_defragment_user {
unsigned long long start;
// unsigned long long len;
};
main()
{
struct f2fs_defragment_user *df;
df = malloc();
ioctl(fd, F2FS_IOC_DEFRAGMENT, df);
}
kernel:
f2fs_ioc_defragment()
{
...
if (copy_from_user(&range, (struct f2fs_defragment __user *)arg,
sizeof(range)))
return -EFAULT;
}
Is that a common issue?
Thanks,
>
> Thanks,
>
>>
>> - Eric
>> .
>>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
>
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
