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 <ebigg...@google.com>
>>>>
>>>> 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 <ebigg...@google.com>
>>>
>>> Anyway, it makes sense to me.
>>>
>>> Reviewed-by: Chao Yu <yuch...@huawei.com>
>>>
>>
>> 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
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to