diff mbox series

libselinux: silence -Wstringop-overflow warning from gcc 10.3.1

Message ID 20210430193702.42974-1-nicolas.iooss@m4x.org (mailing list archive)
State Accepted
Headers show
Series libselinux: silence -Wstringop-overflow warning from gcc 10.3.1 | expand

Commit Message

Nicolas Iooss April 30, 2021, 7:37 p.m. UTC
When building libselinux on Fedora 33 with gcc 10.3.1, the compiler
reports:

    label_file.c: In function ‘lookup_all.isra’:
    label_file.c:940:4: error: ‘strncpy’ specified bound depends on the
    length of the source argument [-Werror=stringop-overflow=]
      940 |    strncpy(clean_key, key, len - 1);
          |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    label_file.c:927:8: note: length computed here
      927 |  len = strlen(key);
          |        ^~~~~~~~~~~
    cc1: all warnings being treated as errors

As clean_key is the result of malloc(len), there is no issue here. But
using strncpy can be considered as strange, because the size of the
string is already known and the NUL terminator is always added later, in
function ‘lookup_all.isra.

Replace strncpy with memcpy to silence this gcc false-positive warning.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libselinux/src/label_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Petr Lautrbach May 7, 2021, 11:21 a.m. UTC | #1
Nicolas Iooss <nicolas.iooss@m4x.org> writes:

> When building libselinux on Fedora 33 with gcc 10.3.1, the compiler
> reports:
>
>     label_file.c: In function ‘lookup_all.isra’:
>     label_file.c:940:4: error: ‘strncpy’ specified bound depends on the
>     length of the source argument [-Werror=stringop-overflow=]
>       940 |    strncpy(clean_key, key, len - 1);
>           |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     label_file.c:927:8: note: length computed here
>       927 |  len = strlen(key);
>           |        ^~~~~~~~~~~
>     cc1: all warnings being treated as errors
>
> As clean_key is the result of malloc(len), there is no issue here. But
> using strncpy can be considered as strange, because the size of the
> string is already known and the NUL terminator is always added later, in
> function ‘lookup_all.isra.
>
> Replace strncpy with memcpy to silence this gcc false-positive warning.
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>

I wasn't able to reproduce it with gcc-11.0.1-0.7.fc35.x86_64, but it's
indeed reproducible with gcc-10.3.1-1.fc33.x86_64 and this patch fixes
it.

Acked-by: Petr Lautrbach <plautrba@redhat.com>


> ---
>  libselinux/src/label_file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index 726394ca4332..cfce23e0119e 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -909,7 +909,7 @@ static const struct spec **lookup_all(struct selabel_handle *rec,
>  			if (!clean_key)
>  				goto finish;
>  
> -			strncpy(clean_key, key, len - 1);
> +			memcpy(clean_key, key, len - 1);
>  		}
>  
>  		clean_key[len - 1] = '\0';
> -- 
> 2.31.0
Petr Lautrbach May 12, 2021, 8:08 a.m. UTC | #2
Petr Lautrbach <plautrba@redhat.com> writes:

> Nicolas Iooss <nicolas.iooss@m4x.org> writes:
>
>> When building libselinux on Fedora 33 with gcc 10.3.1, the compiler
>> reports:
>>
>>     label_file.c: In function ‘lookup_all.isra’:
>>     label_file.c:940:4: error: ‘strncpy’ specified bound depends on the
>>     length of the source argument [-Werror=stringop-overflow=]
>>       940 |    strncpy(clean_key, key, len - 1);
>>           |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>     label_file.c:927:8: note: length computed here
>>       927 |  len = strlen(key);
>>           |        ^~~~~~~~~~~
>>     cc1: all warnings being treated as errors
>>
>> As clean_key is the result of malloc(len), there is no issue here. But
>> using strncpy can be considered as strange, because the size of the
>> string is already known and the NUL terminator is always added later, in
>> function ‘lookup_all.isra.
>>
>> Replace strncpy with memcpy to silence this gcc false-positive warning.
>>
>> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> I wasn't able to reproduce it with gcc-11.0.1-0.7.fc35.x86_64, but it's
> indeed reproducible with gcc-10.3.1-1.fc33.x86_64 and this patch fixes
> it.
>
> Acked-by: Petr Lautrbach <plautrba@redhat.com>
>

Merged

>> ---
>>  libselinux/src/label_file.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
>> index 726394ca4332..cfce23e0119e 100644
>> --- a/libselinux/src/label_file.c
>> +++ b/libselinux/src/label_file.c
>> @@ -909,7 +909,7 @@ static const struct spec **lookup_all(struct selabel_handle *rec,
>>  			if (!clean_key)
>>  				goto finish;
>>  
>> -			strncpy(clean_key, key, len - 1);
>> +			memcpy(clean_key, key, len - 1);
>>  		}
>>  
>>  		clean_key[len - 1] = '\0';
>> -- 
>> 2.31.0
diff mbox series

Patch

diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
index 726394ca4332..cfce23e0119e 100644
--- a/libselinux/src/label_file.c
+++ b/libselinux/src/label_file.c
@@ -909,7 +909,7 @@  static const struct spec **lookup_all(struct selabel_handle *rec,
 			if (!clean_key)
 				goto finish;
 
-			strncpy(clean_key, key, len - 1);
+			memcpy(clean_key, key, len - 1);
 		}
 
 		clean_key[len - 1] = '\0';