[PATCH v2] middle-end/104854: Limit strncmp overread warnings

Siddhesh Poyarekar siddhesh@gotplt.org
Thu Mar 17 17:22:59 GMT 2022


On 17/03/2022 22:16, Jeff Law wrote:
>     #include <string.h>
>     char a[] = "abc";
>     char b[] = "abcd";
> 
>     int f (void)
>     {
>        return strncmp (a, b, 8);
>     }
> 
>     where I get
> 
>     t.c:7:10: warning: ‘strncmp’ specified bound 8 exceeds source size 5
>     [-Wstringop-overread]
>          7 |   return strncmp (a, b, 8);   // -Wstringop-overread
>            |          ^~~~~~~~~~~~~~~~~
> 
>     even without -Wall.  strncmp sees that a[3] is '\0' so it stops
>     comparing
>     and there's no UB.
> 
> This one is a clear case where warning is bad.   Both arguments are 
> constant and we can determine they are NUL terminated and an overread 
> will never occur.  No deep analysis really needed here.
> 
> THe far more interesting case in my mind is when one or both arguments 
> have an unknown NUL termination state.  I could argue either side of 
> that case.  I lean towards warning but I understand that opinions differ 
> and my priorities have moved away from distro-level issues, so 
> identifying code that needs a careful review for correctness, 
> particularly old or security sensitive code, has become a much lower 
> priority for me.   Combine that with the fact that we're really just 
> dealing with over-reads here, I can support whatever the broadest 
> consensus is.

Actually in the above reproducer a and b are not const, so this is in 
fact the case where the NUL termination state of the strings is in 
theory unknown.  From the distro level (and in general for applications) 
the question is how common this is and I gathered from a Red Hat 
internal conversation that it's not uncommon.  However David pointed out 
that I need to share more specific examples to quantify this, so I need 
to work on that.  I'll share an update once I have it.

One case I am aware of is the pmix package in Fedora/RHEL, which has the 
following warning:

pmix-3.2.3/examples/alloc.c: scope_hint: In function 'main'
pmix-3.2.3/examples/alloc.c:179:31: warning[-Wstringop-overread]: 
'PMIx_Get' reading 512 bytes from a region of size 15
   179 |     if (PMIX_SUCCESS != (rc = PMIx_Get(&proc, PMIX_UNIV_SIZE, 
NULL, 0, &val))) {
       | 
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pmix-3.2.3/examples/alloc.c:179:31: note: referencing argument 2 of type 
'const char *'
pmix-3.2.3/examples/alloc.c:33: included_from: Included from here.
pmix-3.2.3/include/pmix.h:203:27: note: in a call to function 'PMIx_Get'
   203 | PMIX_EXPORT pmix_status_t PMIx_Get(const pmix_proc_t *proc, 
const pmix_key_t key,
       |                           ^~~~~~~~
   177|       PMIX_PROC_CONSTRUCT(&proc);
   178|       PMIX_LOAD_PROCID(&proc, myproc.nspace, PMIX_RANK_WILDCARD);
   179|->     if (PMIX_SUCCESS != (rc = PMIx_Get(&proc, PMIX_UNIV_SIZE, 
NULL, 0, &val))) {
   180|           fprintf(stderr, "Client ns %s rank %d: PMIx_Get 
universe size failed: %d\n", myproc.nspace, myproc.rank, rc);
   181|           goto done;

which is due to PMIx_Get calling strncmp a few levels within with 
non-const strings and a max size of 512 (the maximum size that a key 
could be; AFAICT it's the size of the buffer into which the key gets 
written out), where the strings are always NULL terminated.

Thanks,
Siddhesh


More information about the Gcc-patches mailing list