linux-kernel - Re: [PATCH v1 1/1] s390: Use string_upper() instead of open coded variant

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] [day] [month] [year] [list]

Message-ID: <YWVRbQrDk7jtYApD@osiris>
Date:   Tue, 12 Oct 2021 11:12:13 +0200
From:   Heiko Carstens <hca@...ux.ibm.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ibm.com>
Subject: Re: [PATCH v1 1/1] s390: Use string_upper() instead of open coded
 variant

On Tue, Oct 12, 2021 at 08:04:58AM +0000, David Laight wrote:
> From: Heiko Carstens
> > Sent: 11 October 2021 11:10
> > 
> > On Mon, Oct 11, 2021 at 08:21:15AM +0000, David Laight wrote:
> > > ...
> > > > > +	 * This snprintf() call does two things:
> > > > > +	 * - makes a NUL-terminated copy of the input string
> > > > > +	 * - pads it with spaces
> > > > > +	 */
> > > > > +	snprintf(tmp, sizeof(tmp), "%s        ", name);
> > > >
> > > > I can't say I like code where I have to count spaces in order to
> > > > verify if the code is actually correct.
> > >
> > > What it wrong with "%-8.8s" ?
> > 
> > There's nothing wrong with it, except lack of imagination on my side ;)
> > Andy, care to to send a separate patch just for extmem.c?
> 
> Are any of the snprintf() versions actually correct at all?
> The implication of the comment is that the input string might
> not be NUL terminated - in which case it shouldn't be passed
> to snprintf().
> I don't think you can assume that the format processing doesn't
> do a strlen() of any %s argument - even if a maximum field
> width is given.

The input string is a NUL terminated ASCII string. The output string
is not. It is used to communicate with a hypervisor, which expects an
eight character EBCDIC non NUL terminated name, where the name is
either eight characters long or filled up with spaces.
So using snprintf here should be fine. On the other hand I really
don't see a pressing need to change anything here.