Yongji Xie <xieyon...@bytedance.com> writes:
> On Tue, Sep 20, 2022 at 7:25 PM Markus Armbruster <arm...@redhat.com> wrote:
>>
>> Philippe Mathieu-Daudé <f4...@amsat.org> writes:
>>
>> > GCC 8 added a -Wstringop-truncation warning:
>> >
>> > The -Wstringop-truncation warning added in GCC 8.0 via r254630 for
>> > bug 81117 is specifically intended to highlight likely unintended
>> > uses of the strncpy function that truncate the terminating NUL
>> > character from the source string.
>> >
>> > Here the next line indeed unconditionally zeroes the last byte, so
>> > we can call strncpy() on the buffer size less the last byte.
>>
>> Actually, the buffer is all zero to begin with, so we could do this even
>> without the next line's assignment.
>>
>
> Yes, I think we can remove the next line's assignment.
>
>> > This
>> > fixes when using gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0:
>> >
>> > [42/666] Compiling C object
>> > subprojects/libvduse/libvduse.a.p/libvduse.c.o
>> > FAILED: subprojects/libvduse/libvduse.a.p/libvduse.c.o
>> > cc -m64 -mcx16 -Isubprojects/libvduse/libvduse.a.p
>> > -Isubprojects/libvduse -I../../subprojects/libvduse [...] -o
>> > subprojects/libvduse/libvduse.a.p/libvduse.c.o -c
>> > ../../subprojects/libvduse/libvduse.c
>> > In file included from /usr/include/string.h:495,
>> > from ../../subprojects/libvduse/libvduse.c:24:
>> > In function ‘strncpy’,
>> > inlined from ‘vduse_dev_create’ at
>> > ../../subprojects/libvduse/libvduse.c:1312:5:
>> > /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
>> > ‘__builtin_strncpy’ specified bound 256 equals destination size
>> > [-Werror=stringop-truncation]
>> > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos
>> > (__dest));
>> > |
>> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > cc1: all warnings being treated as errors
>> > ninja: build stopped: cannot make progress due to previous errors.
>> >
>> > Fixes: d9cf16c0be ("libvduse: Replace strcpy() with strncpy()")
>> > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>>
>> The subject feels a bit too alarming to me. This patch suppresses a
>> warning, no less, no more. Behavior doesn't change. Perhaps
>>
>> libvduse: Avoid warning about dangerous use of strncpy()
>>
>> > ---
>> > Cc: Xie Yongji <xieyon...@bytedance.com>
>> > Cc: Markus Armbruster <arm...@redhat.com>
>> > Cc: Kevin Wolf <kw...@redhat.com>
>> >
>> > RFC: Any better idea? We can't use strpadcpy() because libvduse
>> > doesn't depend on QEMU.
>>
>> There's no need for padding: the destination calloc'ed. So, pstrcpy()
>> would do, but it's just as unavailable. Can we use GLib? There's
>> g_strlcpy().
>>
>> Outside this patch's scope: is silent truncation what we want?
>>
>
> Actually silent truncation would not happen since we called
> vduse_name_is_invalid() before.
>
> static inline bool vduse_name_is_invalid(const char *name)
> {
> return strlen(name) >= VDUSE_NAME_MAX || strstr(name, "..");
> }
Ah, so even strcpy() would be safe (but might trigger a compiler
warning).
Thanks!
