linux-kernel - RE: Writing to a const pointer: is this supposed to happen?

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: <0c2bda4dd9e64a019d69339cf9054586@AcuMS.aculab.com>
Date:   Fri, 3 Jul 2020 08:13:08 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Kars Mulder' <kerneldev@...smulder.nl>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Pavel Machek <pavel@....cz>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kai-Heng Feng <kai.heng.feng@...onical.com>
Subject: RE: Writing to a const pointer: is this  supposed to happen?

From: Kars Mulder
> Sent: 02 July 2020 22:48
> 
> On Thursday, July 02, 2020 09:55 CEST, David Laight wrote:
> > Hmm... sscanf() is also horrid.
> > Surprisingly difficult to use correctly.
> >
> > It is usually best to use strchr() (and maybe str[c]scn())
> > to parse strings.
> > For numbers use whatever the kernels current 'favourite' implementation
> > of strtoul() is called.
> 
> I thought that using sscanf would clean up the code a bit compared to
> several haphazard calls, but I can see your point about sscanf being
> difficult to use correctly.
> 
> The kernel functions kstrtou16 seem to expect a null-terminated string
> as argument. Since there are no null-bytes after the numbers we want to
> parse, it becomes necessary to copy at least part of the strings to a
> buffer.

There ought to be one that returns a pointer to the first character
that isn't converted - but I'm no expert on the full range of these
functions.

> If we're copying strings to buffers anyway, I think the simplest
> solution would be to just kstrdup the entire parameter and not touch
> the rest of the string parsing code. This has the disadvantage of
> having an extra memory allocation to keep track of.
> 
> Since the parameter is currently restricted to 128 characters at
> most, it may alternatively be possible to copy the parameter to
> a 128-byte buffer on the stack. This has the advantage of having
> to keep track of one less memory allocation, but the disadvantage
> of using 128 bytes more stack space; I'm not sure whether that's
> acceptable.

The problem with strdup() is you get the extra (unlikely) failure path.
128 bytes of stack won't be a problem if the function is (essentially)
a leaf.
Deep stack use is actually likely to be in the bowels of printf())
inside an obscure error path.
Many years ago (about 1984) I parsed the object code of a program
to find the deepest stack use (no recursion and no function pointers)
so we could set the stack sizes correctly - there wasn't enough
memory to do it properly!

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)