[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[msebor at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

--- Comment #2 from Martin Sebor <msebor at gmail dot com> ---
An updated patch:
https://sourceware.org/pipermail/libc-alpha/2020-April/113503.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[msc at linux dot ibm.com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

Matheus Castanho <msc at linux dot ibm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msc at linux dot ibm.com

--- Comment #3 from Matheus Castanho <msc at linux dot ibm.com> ---
This seems to have already been merged as
06febd8c6705c816b2f32ee7aa1f4c0184b05248 (though not reflected here).

I believe this is causing an issue when building binutils (I was not able to
come up with a shorter reproducer yet, sorry):

/home/mscastanho/AT/next/tmp/build/at14.0-0-alpha.debian-10_ppc64le_ppc64le/sources/binutils/gas/read.c:
In function ‘read_symbol_name’:
/home/mscastanho/AT/next/tmp/build/at14.0-0-alpha.debian-10_ppc64le_ppc64le/sources/binutils/gas/read.c:1687:11:
error: argument 1 is null but the corresponding size argument 3 range is [128,
9223372036854775807] [-Werror=nonnull]
 1687 |       if (mbstowcs (NULL, name, len) == (size_t) -1)
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from
/home/mscastanho/AT/next/tmp/build/at14.0-0-alpha.debian-10_ppc64le_ppc64le/sources/binutils/gas/as.h:58,
                 from
/home/mscastanho/AT/next/tmp/build/at14.0-0-alpha.debian-10_ppc64le_ppc64le/sources/binutils/gas/read.c:33:
/home/mscastanho/AT/next/opt/at-next-14.0-0-alpha/include/stdlib.h:933:15:
note: in a call to function ‘mbstowcs’ declared with attribute ‘write_only (1,
3)’
  933 | extern size_t mbstowcs (wchar_t *__restrict  __pwcs,
      |               ^~~~~~~~
cc1: all warnings being treated as errors


So GCC is complaining because arg 1 of mbstowcs is NULL but arg 3 is a positive
value. But from the function's manpage: "If dest [arg1] is NULL, n is ignored,
and the conversion proceeds..."

So is there a chance this is a valid usage of the function that was not
contemplated by this patch?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[msebor at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

Martin Sebor <msebor at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
           Severity|normal                      |enhancement
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #4 from Martin Sebor <msebor at gmail dot com> ---
The attribute has been added.

I added the attribute to most of the APIs based on the requirements in the
standards (C11 and POSIX).  I also checked a few  for extensions described in
the Glibc documentation and at least one in the Linux documentation project,
but I don't think I looked in the latter for mbstowcs.

C11 requires the first argument to mbstowcs to be an array (i.e., non-null).  I
don't see an extension documented in the Glibc manual to allow null, but the
Linux man page (http://man7.org/linux/man-pages/man3/mbstowcs.3.html) does.

I'll leave it to the Glibc maintainers to decide what they want to do here.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |2.32

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

--- Comment #5 from Florian Weimer <fweimer at redhat dot com> ---
(In reply to Martin Sebor from comment #4)
> The attribute has been added.
> 
> I added the attribute to most of the APIs based on the requirements in the
> standards (C11 and POSIX).  I also checked a few  for extensions described
> in the Glibc documentation and at least one in the Linux documentation
> project, but I don't think I looked in the latter for mbstowcs.
> 
> C11 requires the first argument to mbstowcs to be an array (i.e., non-null).
> I don't see an extension documented in the Glibc manual to allow null, but
> the Linux man page (http://man7.org/linux/man-pages/man3/mbstowcs.3.html)
> does.

POSIX has an XSI extension which makes the NULL argument valid:

“
[XSI] [Option Start]  If pwcs is a null pointer, mbstowcs() shall return the
length required to convert the entire array regardless of the value of n, but
no values are stored. [Option End]
”

So this needs to be fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[msebor at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

--- Comment #6 from Martin Sebor <msebor at gmail dot com> ---
Thanks, I clearly missed the XSI extension.

The most straightforward fix to avoid the warning is to remove the attribute
from mbstowcs.

Longer term though, unless it's declared with attribute nonnull, perhaps the
attribute should allow the pointer to be null regardless of the size.  That's
what it does now when the size argument isn't specified.  When the size is
specified, the attribute it's modeled on functions like memcpy, but that may
not be the most flexible model.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

--- Comment #7 from Florian Weimer <fweimer at redhat dot com> ---
(In reply to Martin Sebor from comment #6)
> Thanks, I clearly missed the XSI extension.
> 
> The most straightforward fix to avoid the warning is to remove the attribute
> from mbstowcs.
> 
> Longer term though, unless it's declared with attribute nonnull, perhaps the
> attribute should allow the pointer to be null regardless of the size. 
> That's what it does now when the size argument isn't specified.  When the
> size is specified, the attribute it's modeled on functions like memcpy, but
> that may not be the most flexible model.

It may already be possible today to implement the desired NULL behavior with an
inline wrapper that calls different aliases as needed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[jakub at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

Jakub Jelinek <jakub at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at redhat dot com

--- Comment #8 from Jakub Jelinek <jakub at redhat dot com> ---
getcwd has the same problem, see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96832

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25219] improve out-of-bounds checking with GCC 10 attribute access

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25219

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://sourceware.org/bugz
                   |                            |illa/show_bug.cgi?id=26545

-- 
You are receiving this mail because:
You are on the CC list for the bug.