| Message ID | 20190118193248.535-1-malat@debian.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | video/hdmi: Change strncpy() into memcpy() in hdmi_spd_infoframe_init | expand |
On Fri, Jan 18, 2019 at 8:51 PM Joe Perches <joe@perches.com> wrote: > > On Fri, 2019-01-18 at 20:32 +0100, Mathieu Malaterre wrote: > > Using strncpy() is less than perfect since the destination buffers do not > > need to be NUL terminated. Replace strncpy() with memcpy() to address a > > warning triggered by gcc using W=1 and optimize the code a bit. > > > > This commit removes the following warnings: > > > > drivers/video/hdmi.c:234:2: warning: 'strncpy' specified bound 8 equals destination size [-Wstringop-truncation] > > drivers/video/hdmi.c:235:2: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation] > [] > > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c > [] > > @@ -231,8 +231,8 @@ int hdmi_spd_infoframe_init(struct hdmi_spd_infoframe *frame, > > frame->version = 1; > > frame->length = HDMI_SPD_INFOFRAME_SIZE; > > > > - strncpy(frame->vendor, vendor, sizeof(frame->vendor)); > > - strncpy(frame->product, product, sizeof(frame->product)); > > + memcpy(frame->vendor, vendor, sizeof(frame->vendor)); > > + memcpy(frame->product, product, sizeof(frame->product)); > > This is not good. > > vendor can be any location and shorter than sizeof(frame->vendor) > so this can copy from invalid memory locations. Ah right. I did not realize that and know I see the call with "Intel", will re-spin. > You probably want strscpy. Right. > This is called with at least "mediatek" and "broadcom", so perhaps > it's better still to change the struct vendor size to something a > bit larger. Maybe change vendor[8] to vendor[16]; Looks like 8 bytes is required for call like hdmi_spd_infoframe_unpack() > include/linux/hdmi.h:struct hdmi_spd_infoframe { > include/linux/hdmi.h- enum hdmi_infoframe_type type; > include/linux/hdmi.h- unsigned char version; > include/linux/hdmi.h- unsigned char length; > include/linux/hdmi.h- char vendor[8]; > include/linux/hdmi.h- char product[16]; > include/linux/hdmi.h- enum hdmi_spd_sdi sdi; > include/linux/hdmi.h-}; > >
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index 799ae49774f5..553c39ac8f9e 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -231,8 +231,8 @@ int hdmi_spd_infoframe_init(struct hdmi_spd_infoframe *frame, frame->version = 1; frame->length = HDMI_SPD_INFOFRAME_SIZE; - strncpy(frame->vendor, vendor, sizeof(frame->vendor)); - strncpy(frame->product, product, sizeof(frame->product)); + memcpy(frame->vendor, vendor, sizeof(frame->vendor)); + memcpy(frame->product, product, sizeof(frame->product)); return 0; }
Using strncpy() is less than perfect since the destination buffers do not need to be NUL terminated. Replace strncpy() with memcpy() to address a warning triggered by gcc using W=1 and optimize the code a bit. This commit removes the following warnings: drivers/video/hdmi.c:234:2: warning: 'strncpy' specified bound 8 equals destination size [-Wstringop-truncation] drivers/video/hdmi.c:235:2: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation] Signed-off-by: Mathieu Malaterre <malat@debian.org> --- drivers/video/hdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)