diff mbox series

[v2] mmc-utils: fix overlapping with strncpy

Message ID 20181024110047.17274-1-peron.clem@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] mmc-utils: fix overlapping with strncpy | expand

Commit Message

Clément Péron Oct. 24, 2018, 11 a.m. UTC
GCC 8.2 warns about an overlapping using strncpy.

Replace strncpy with a memmove to avoid this issue.

In file included from /usr/include/string.h:494,
                 from lsmmc.c:46:
In function ‘strncpy’,
    inlined from ‘read_file’ at lsmmc.c:356:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ accessing 4096 bytes at offsets 0 and 1 overlaps 4095 bytes at offset 1 [-Werror=restrict]
   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:36: lsmmc.o] Error 1

Signed-off-by: Clément Péron <peron.clem@gmail.com>
---

v2:
	repost with Chris Ball

 lsmmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Clément Péron Nov. 5, 2018, 4:13 p.m. UTC | #1
Hi Chris,

Could you have a look at this patch ?

Thanks,
Clement

On Wed, 24 Oct 2018 at 13:00, Clément Péron <peron.clem@gmail.com> wrote:
>
> GCC 8.2 warns about an overlapping using strncpy.
>
> Replace strncpy with a memmove to avoid this issue.
>
> In file included from /usr/include/string.h:494,
>                  from lsmmc.c:46:
> In function ‘strncpy’,
>     inlined from ‘read_file’ at lsmmc.c:356:3:
> /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ accessing 4096 bytes at offsets 0 and 1 overlaps 4095 bytes at offset 1 [-Werror=restrict]
>    return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile:36: lsmmc.o] Error 1
>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>
> v2:
>         repost with Chris Ball
>
>  lsmmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lsmmc.c b/lsmmc.c
> index c4faa00..bcb854d 100644
> --- a/lsmmc.c
> +++ b/lsmmc.c
> @@ -353,7 +353,7 @@ char *read_file(char *name)
>                 line[strlen(line) - 1] = '\0';
>
>         while (isspace(line[0]))
> -               strncpy(&line[0], &line[1], sizeof(line));
> +               memmove(&line[0], &line[1], sizeof(line)-1);
>
>         return strdup(line);
>  }
> --
> 2.19.1
>
Hans-Christian Noren Egtvedt Nov. 5, 2018, 7:05 p.m. UTC | #2
Around Mon 05 Nov 2018 17:13:43 +0100 or thereabout, Clément Péron wrote:

Hello,

> Could you have a look at this patch ?
> 
> Thanks,
> Clement
> 
> On Wed, 24 Oct 2018 at 13:00, Clément Péron <peron.clem@gmail.com> wrote:
>>
>> GCC 8.2 warns about an overlapping using strncpy.
>>
>> Replace strncpy with a memmove to avoid this issue.
>>
>> In file included from /usr/include/string.h:494,
>>                  from lsmmc.c:46:
>> In function ‘strncpy’,
>>     inlined from ‘read_file’ at lsmmc.c:356:3:
>> /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ accessing 4096 bytes at offsets 0 and 1 overlaps 4095 bytes at offset 1 [-Werror=restrict]
>>    return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> make: *** [Makefile:36: lsmmc.o] Error 1
>>
>> Signed-off-by: Clément Péron <peron.clem@gmail.com>

Acked-by: Hans-Christian Noren Egtvedt <egtvedt@samfundet.no>

>> ---
>>
>> v2:
>>         repost with Chris Ball
>>
>>  lsmmc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lsmmc.c b/lsmmc.c
>> index c4faa00..bcb854d 100644
>> --- a/lsmmc.c
>> +++ b/lsmmc.c
>> @@ -353,7 +353,7 @@ char *read_file(char *name)
>>                 line[strlen(line) - 1] = '\0';
>>
>>         while (isspace(line[0]))
>> -               strncpy(&line[0], &line[1], sizeof(line));
>> +               memmove(&line[0], &line[1], sizeof(line)-1);
>>
>>         return strdup(line);
>>  }
>> --
>> 2.19.1

This works well for me at least.
Michael Heimpold Nov. 5, 2018, 7:31 p.m. UTC | #3
Hi,

Am Montag, 5. November 2018, 17:13:43 CET schrieben Sie:
> Hi Chris,
> 
> Could you have a look at this patch ?
> 
> Thanks,
> Clement
> 
> On Wed, 24 Oct 2018 at 13:00, Clément Péron <peron.clem@gmail.com> wrote:
> > GCC 8.2 warns about an overlapping using strncpy.
> > 
> > Replace strncpy with a memmove to avoid this issue.

while memmove does not suffer from this limitation, I don't see any reason
to move the whole string at all - furthermore multiple times in worst case.
Since we leave the function with a strdup() call, we could just pass an
adjusted start pointer to strdup() without the need to move things before.

I sent a patch "lsmmc: rework string trimming" which follows this approach.
(Yes, sorry, I did not checked the ML before - so I didn't noticed your 
patches... )

Regards
Michael 

> > 
> > In file included from /usr/include/string.h:494,
> > 
> >                  from lsmmc.c:46:
> > In function ‘strncpy’,
> > 
> >     inlined from ‘read_file’ at lsmmc.c:356:3:
> > /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
> > ‘__builtin_strncpy’ accessing 4096 bytes at offsets 0 and 1 overlaps 4095
> > bytes at offset 1 [-Werror=restrict]> 
> >    return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >    
> >           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > cc1: all warnings being treated as errors
> > make: *** [Makefile:36: lsmmc.o] Error 1
> > 
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> > 
> > v2:
> >         repost with Chris Ball
> >  
> >  lsmmc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lsmmc.c b/lsmmc.c
> > index c4faa00..bcb854d 100644
> > --- a/lsmmc.c
> > +++ b/lsmmc.c
> > @@ -353,7 +353,7 @@ char *read_file(char *name)
> > 
> >                 line[strlen(line) - 1] = '\0';
> >         
> >         while (isspace(line[0]))
> > 
> > -               strncpy(&line[0], &line[1], sizeof(line));
> > +               memmove(&line[0], &line[1], sizeof(line)-1);
> > 
> >         return strdup(line);
> >  
> >  }
> > 
> > --
> > 2.19.1
diff mbox series

Patch

diff --git a/lsmmc.c b/lsmmc.c
index c4faa00..bcb854d 100644
--- a/lsmmc.c
+++ b/lsmmc.c
@@ -353,7 +353,7 @@  char *read_file(char *name)
 		line[strlen(line) - 1] = '\0';
 
 	while (isspace(line[0]))
-		strncpy(&line[0], &line[1], sizeof(line));
+		memmove(&line[0], &line[1], sizeof(line)-1);
 
 	return strdup(line);
 }