[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() inste
From: |
Chen Gang |
Subject: |
Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf() |
Date: |
Wed, 05 Feb 2014 07:44:30 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 |
On 02/05/2014 12:18 AM, Aneesh Kumar K.V wrote:
> Chen Gang <address@hidden> writes:
>
>> On 02/04/2014 07:06 PM, Daniel P. Berrange wrote:
>>> On Tue, Feb 04, 2014 at 07:02:18PM +0800, Chen Gang wrote:
>>>> On 02/03/2014 06:39 PM, Chen Gang wrote:
>>>>> On 02/03/2014 06:34 PM, Daniel P. Berrange wrote:
>>>>>> On Mon, Feb 03, 2014 at 06:00:42PM +0800, Chen Gang wrote:
>>>>>>> We can not assume "'path' + 'ctx->fs_root'" must be less than MAX_PATH,
>>>>>>> so need use snprintf() instead of sprintf().
>>>>>>>
>>>>>>> And also recommend to use ARRAY_SIZE instead of hard code macro for an
>>>>>>> array size in snprintf().
>>>>>>
>>>>>> In the event that there is overflow this will cause the data to be
>>>>>> truncated, potentially causing QEMU to access the wrong file on the
>>>>>> host. Both snprintf and sprintf are really bad because of their
>>>>>> use of fixed buffers. Better to change it to g_strdup_printf which
>>>>>> dynamically allocates buffers.
>>>>>>
>>>>
>>>> After check the details, I guess we can not change to g_strdup_printf or
>>>> others (e.g. v9fs_string_*).
>>>>
>>>> v9fs need use "mkdir, remove ..." which have MAX_PATH limitation. So if
>>>> the combined path is longer than MAX_PATH, before it passes to "mkdir,
>>>> remove ...", it has to be truncated just like what rpath() has done.
>>>
>>> I don't believe you are correct there. Those functions should
>>> return "errno == ENAMETOOLONG - pathname was too long". The
>>> MAX_PATH constant is not even required to exist in POSIX, so
>>> I would not expect the spec to mandate anything about MAX_PATH
>>> in relation to those functions.
>>>
>>
>> So the original author of v9fs will use truncation instead of return
>> failure to upper users.
>
>
> That is a bug. The snprintf usage with PATH_MAX is to prevent buffer
> overflow and not to truncate. I guess we should fix path handling
> and propagate error correctly.
>
> -aneesh
>
OK, thank you for your opinion and confirmation. I will/should send
patch v2 for it (use 'unlimited' path and propagate error correctly).
And excuse me, I have no enough time to focus on it, so I plan to send
patch v2 for reviewing within this month (2014-02-28). If we can not
bear this time point, please help send patch for it, thanks.
And also excuse me, I am a newbie for 9pfs, also a newbie for qemu (I
found it by reading source code), the patch v2 needs a test for 9pfs, so
welcome any suggestions/informations about 9pfs test.
Thanks
--
Chen Gang
Open, share and attitude like air, water and life which God blessed
- [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Chen Gang, 2014/02/03
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Daniel P. Berrange, 2014/02/03
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Chen Gang, 2014/02/03
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Chen Gang, 2014/02/04
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Daniel P. Berrange, 2014/02/04
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Chen Gang, 2014/02/04
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Aneesh Kumar K.V, 2014/02/04
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(),
Chen Gang <=
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Chen Gang, 2014/02/15
- [Qemu-devel] [PATCH] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation, Chen Gang, 2014/02/22
- Re: [Qemu-devel] [PATCH] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation, Chen Gang, 2014/02/23
- Re: [Qemu-devel] [PATCH] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation, Markus Armbruster, 2014/02/24
- Re: [Qemu-devel] [PATCH] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation, Gang Chen, 2014/02/24
- Re: [Qemu-devel] [PATCH] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation, Markus Armbruster, 2014/02/24
- Re: [Qemu-devel] [PATCH] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation, Chen Gang, 2014/02/27
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Eric Blake, 2014/02/04
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Markus Armbruster, 2014/02/04
- Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf(), Eric Blake, 2014/02/04