Re: linux-next: build warnings from Linus' tree

[Adam Borowski]

On Sun, Aug 19, 2018 at 04:21:57PM -0700, Linus Torvalds wrote:
> On Sun, Aug 19, 2018 at 3:13 PM Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
> >
> > Today's linux-next build (powerpc ppc64_defconfig) produced these
> > warnings:
> >
> > fs/cifs/cifssmb.c:605:3: warning: 'strncpy' writing 16 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=]
> >    strncpy(pSMB->DialectsArray+count, protocols[i].name, 16);
> >
> > Presumably caused by my update to gcc 8.2.0.
> 
> Yeah. There are some patches to mark some arrays as non-strings to get
> rid of these, but we'll see. Maybe we'll just disable the new gcc
> warning if it causes more pain than it is worth.

Every single use of strncpy() for a C string is either a bug, inefficiency,
or both.  In this particular case the code:

        count = 0;
        for (i = 0; i < CIFS_NUM_PROT; i++) {
                strncpy(pSMB->DialectsArray+count, protocols[i].name, 16);
                count += strlen(protocols[i].name) + 1;
                /* null at end of source and target buffers anyway */
        }

* pointlessly clears 16 bytes in every iteration
* calculates the string's length twice
* there's no protection against buffer overflow anyway

So what is the strncpy() there for, when an unbounded copy would be just as
good?  For other cases, there's a bunch of better functions: strlcpy(),
snprintf(), even strlen()+memcpy(), etc.

Valid uses of strncpy() do exist (such as SCSI structs), but those deal with
fixed-width fields.  Thus, gcc is right for warning for at least some of
misuse of strncpy() for C strings.  The function wasn't designed for them.

(Skipped analysis why strncpy is always a bad choice for C strings.)


Meow!
-- 
âââââââ What Would Jesus Do, MUD/MMORPG edition:
âââââââ â multiplay with an admin char to benefit your mortal [Mt3:16-17]
âââââââ â abuse item cloning bugs [Mt14:17-20, Mt15:34-37]
âââââââ â use glitches to walk on water [Mt14:25-26]