linux-kernel - Re: [PATCH V2] auxdisplay: use correct string length

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180212171137.GA31601@1wt.eu>
Date:   Mon, 12 Feb 2018 18:11:37 +0100
From:   Willy Tarreau <w@....eu>
To:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc:     Xiongfeng Wang <wangxiongfeng2@...wei.com>,
        Dan <dan.carpenter@...cle.com>, Arnd Bergmann <arnd@...db.de>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2] auxdisplay: use correct string length

On Mon, Feb 12, 2018 at 01:53:57PM +0100, Miguel Ojeda wrote:
> > diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
> > index ea7869c..d288900 100644
> > --- a/drivers/auxdisplay/panel.c
> > +++ b/drivers/auxdisplay/panel.c
> > @@ -1506,10 +1506,10 @@ static struct logical_input *panel_bind_key(const char *name, const char *press,
> >         key->rise_time = 1;
> >         key->fall_time = 1;
> >
> > -       strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str));
> > -       strncpy(key->u.kbd.repeat_str, repeat, sizeof(key->u.kbd.repeat_str));
> > +       strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str) - 1);
> > +       strncpy(key->u.kbd.repeat_str, repeat, sizeof(key->u.kbd.repeat_str) - 1);
> >         strncpy(key->u.kbd.release_str, release,
> > -               sizeof(key->u.kbd.release_str));
> > +               sizeof(key->u.kbd.release_str) - 1);
> 
> Are you sure about this patch? `kbd` says "strings can be non null-terminated".
> 
> Willy, maybe those should just be memcpy()s? (unless the remaining
> bytes, if any, must be 0).

For me this seems to be the result of yet another very stupid gcc warning
trying to dissuade us from using well defined fonctions... it's unimaginable
how gcc warnings have become stupid and irrelevant since its developers
stopped using C to write it :-(

If you want to work around this wrong warning, probably that increasing the
destination storage size by one and adding -1 to strncpy() would shut it up
but that really becomes quite annoying to have to modify code and storage
just to shut down a dumbass compiler trying to be smart.

Willy