[PATCH] more out of bounds checking improvements

[PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 2731 bytes --]

The attached patch continues the improvements to out of bounds
checking(*) by decorating more APIs with either attribute access,
or by explicitly providing the array bound in APIs such as
tmpnam() that expect arrays of some minimum size as arguments.
(The latter feature is new in GCC 11.)

The only effects of the attribute and/or the array bound is to
check and diagnose calls to the functions that fail to provide
a sufficient number of elements, and the definitions of
the functions that access elements outside the specified bounds.
(There is no interplay with _FORTIFY_SOURCE here yet.)

For example, because the patch specifies the bound in the tmpnam
declaration like so:

    extern char *tmpnam (char[L_tmpnam]) __THROW __wur;

the following call to the function:

    char* warn_tmpnam (void)
    {
      char a[16];
      return tmpnam (a);
    }

triggers the warning below:

t.c: In function ‘warn_tmpnam’:
t.c:8:10: warning: ‘tmpnam’ accessing 20 bytes in a region of size 16 
[-Wstringop-overflow=]
      8 |   return tmpnam (a);
        |          ^~~~~~~~~~
t.c:8:10: note: referencing argument 1 of type ‘char *’
In file included from ../include/stdio.h:14,
                   from test-access-warn.c:1:
../libio/stdio.h:187:14: note: in a call to function ‘tmpnam’
    187 | extern char *tmpnam (char[L_tmpnam]) __THROW __wur;
        |              ^~~~~~

Unlike the [static N] notation, the plain array notation doesn't
require the argument to be nonnull.

Besides attribute access, the change adds attribute nonnull to
the readv and writev functions in misc/sys/uio.h.  The functions
don't necessarily access the array elements when their count is
zero but neither POSIX nor the Linux manual document this so it
seems appropriate to warn.

The patch introduces the _L_tmpnam macro to avoid polluting
the POSIX <unistd.h> namespace with L_tmpnam when the latter is
only supposed to be defined in <stdio.h>.  This in turn causes
the a number of POSIX conformance test failures that I haven't
been able to figure how to deal with and need some help with.

In file included from ../include/unistd.h:2,
                  from /tmp/tmpzm39v4n3/test.c:1:
../posix/unistd.h:1159:32: error: ‘_L_ctermid’ undeclared here (not in a 
function)
  extern char *ctermid (char __s[_L_ctermid]) __THROW
                                 ^~~~~~~~~~

I expected adding the new macros to stdio-common/stdio_lim.h.in
would do the trick but clearly something else is needed and I'm
at a lost as to what that might be.  I haven't been able to find
my way out of the maze of scripts and makefiles that tie all this
together.

Thanks
Martin

[*] https://sourceware.org/pipermail/libc-alpha/2020-April/113503.html

[-- Attachment #2: glibc-attr-access.diff --]
[-- Type: text/x-patch, Size: 19641 bytes --]

diff --git a/inet/if_index.c b/inet/if_index.c
index ddef419230..36f1806ac1 100644
--- a/inet/if_index.c
+++ b/inet/if_index.c
@@ -31,7 +31,7 @@ libc_hidden_weak (if_nametoindex)
 stub_warning (if_nametoindex)
 
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   __set_errno (ENOSYS);
   return NULL;
diff --git a/io/bits/poll2.h b/io/bits/poll2.h
index dca49717db..8cbedb7542 100644
--- a/io/bits/poll2.h
+++ b/io/bits/poll2.h
@@ -26,13 +26,14 @@ __BEGIN_DECLS
 extern int __REDIRECT (__poll_alias, (struct pollfd *__fds, nfds_t __nfds,
 				      int __timeout), poll);
 extern int __poll_chk (struct pollfd *__fds, nfds_t __nfds, int __timeout,
-		       __SIZE_TYPE__ __fdslen);
+		       __SIZE_TYPE__ __fdslen)
+    __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT (__poll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 					 int __timeout, __SIZE_TYPE__ __fdslen),
 		       __poll_chk)
   __warnattr ("poll called with fds buffer too small file nfds entries");
 
-__fortify_function int
+__fortify_function __attr_access ((__write_only__, 1, 2)) int
 poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
 {
   if (__bos (__fds) != (__SIZE_TYPE__) -1)
@@ -53,7 +54,8 @@ extern int __REDIRECT (__ppoll_alias, (struct pollfd *__fds, nfds_t __nfds,
 				       const __sigset_t *__ss), ppoll);
 extern int __ppoll_chk (struct pollfd *__fds, nfds_t __nfds,
 			const struct timespec *__timeout,
-			const __sigset_t *__ss, __SIZE_TYPE__ __fdslen);
+			const __sigset_t *__ss, __SIZE_TYPE__ __fdslen)
+    __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT (__ppoll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 					  const struct timespec *__timeout,
 					  const __sigset_t *__ss,
@@ -61,7 +63,7 @@ extern int __REDIRECT (__ppoll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 		       __ppoll_chk)
   __warnattr ("ppoll called with fds buffer too small file nfds entries");
 
-__fortify_function int
+__fortify_function __attr_access ((__write_only__, 1, 2)) int
 ppoll (struct pollfd *__fds, nfds_t __nfds, const struct timespec *__timeout,
        const __sigset_t *__ss)
 {
diff --git a/io/sys/poll.h b/io/sys/poll.h
index 857be0f5ac..aa145a1737 100644
--- a/io/sys/poll.h
+++ b/io/sys/poll.h
@@ -51,7 +51,8 @@ __BEGIN_DECLS
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout);
+extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
+    __attr_access ((__write_only__, 1, 2));
 
 #ifdef __USE_GNU
 /* Like poll, but before waiting the threads signal mask is replaced
@@ -62,7 +63,9 @@ extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout);
    __THROW.  */
 extern int ppoll (struct pollfd *__fds, nfds_t __nfds,
 		  const struct timespec *__timeout,
-		  const __sigset_t *__ss);
+		  const __sigset_t *__ss)
+    __attr_access ((__write_only__, 1, 2));
+
 #endif
 
 __END_DECLS
diff --git a/libio/stdio.h b/libio/stdio.h
index 998470943e..59f2ee01ab 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -184,12 +184,12 @@ extern FILE *tmpfile64 (void) __wur;
 #endif
 
 /* Generate a temporary filename.  */
-extern char *tmpnam (char *__s) __THROW __wur;
+extern char *tmpnam (char[L_tmpnam]) __THROW __wur;
 
 #ifdef __USE_MISC
 /* This is the reentrant variant of `tmpnam'.  The only difference is
    that it does not allow S to be NULL.  */
-extern char *tmpnam_r (char *__s) __THROW __wur;
+extern char *tmpnam_r (char __s[L_tmpnam]) __THROW __wur;
 #endif
 
 
@@ -808,13 +808,13 @@ extern int pclose (FILE *__stream);
 
 #ifdef	__USE_POSIX
 /* Return the name of the controlling terminal.  */
-extern char *ctermid (char *__s) __THROW;
+extern char *ctermid (char __s[_L_ctermid]) __THROW;
 #endif /* Use POSIX.  */
 
 
 #if (defined __USE_XOPEN && !defined __USE_XOPEN2K) || defined __USE_GNU
 /* Return the name of the current user.  */
-extern char *cuserid (char *__s);
+extern char *cuserid (char __s[_L_cuserid]);
 #endif /* Use X/Open, but not issue 6.  */
 
 
diff --git a/misc/sys/uio.h b/misc/sys/uio.h
index 26d87c9f34..04ad46d705 100644
--- a/misc/sys/uio.h
+++ b/misc/sys/uio.h
@@ -39,7 +39,7 @@ __BEGIN_DECLS
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t readv (int __fd, const struct iovec *__iovec, int __count)
-  __wur;
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 
 /* Write data pointed by the buffers described by IOVEC, which
    is a vector of COUNT 'struct iovec's, to file descriptor FD.
@@ -50,7 +50,7 @@ extern ssize_t readv (int __fd, const struct iovec *__iovec, int __count)
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
-  __wur;
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 
 
 #ifdef __USE_MISC
@@ -65,7 +65,8 @@ extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count,
-		       __off_t __offset) __wur;
+		       __off_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 
 /* Write data pointed by the buffers described by IOVEC, which is a
    vector of COUNT 'struct iovec's, to file descriptor FD at the given
@@ -77,16 +78,19 @@ extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pwritev (int __fd, const struct iovec *__iovec, int __count,
-			__off_t __offset) __wur;
+			__off_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 
 # else
 #  ifdef __REDIRECT
 extern ssize_t __REDIRECT (preadv, (int __fd, const struct iovec *__iovec,
 				    int __count, __off64_t __offset),
-			   preadv64) __wur;
+			   preadv64)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 extern ssize_t __REDIRECT (pwritev, (int __fd, const struct iovec *__iovec,
 				     int __count, __off64_t __offset),
-			   pwritev64) __wur;
+			   pwritev64)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 #  else
 #   define preadv preadv64
 #   define pwritev pwritev64
@@ -104,7 +108,8 @@ extern ssize_t __REDIRECT (pwritev, (int __fd, const struct iovec *__iovec,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t preadv64 (int __fd, const struct iovec *__iovec, int __count,
-			 __off64_t __offset) __wur;
+			 __off64_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 
 /* Write data pointed by the buffers described by IOVEC, which is a
    vector of COUNT 'struct iovec's, to file descriptor FD at the given
@@ -116,7 +121,8 @@ extern ssize_t preadv64 (int __fd, const struct iovec *__iovec, int __count,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pwritev64 (int __fd, const struct iovec *__iovec, int __count,
-			  __off64_t __offset) __wur;
+			  __off64_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 # endif
 #endif	/* Use misc.  */
 
@@ -125,7 +131,8 @@ extern ssize_t pwritev64 (int __fd, const struct iovec *__iovec, int __count,
 # ifndef __USE_FILE_OFFSET64
 /* Same as preadv but with an additional flag argumenti defined at uio.h.  */
 extern ssize_t preadv2 (int __fp, const struct iovec *__iovec, int __count,
-			__off_t __offset, int ___flags) __wur;
+			__off_t __offset, int ___flags)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 
 /* Same as preadv but with an additional flag argument defined at uio.h.  */
 extern ssize_t pwritev2 (int __fd, const struct iovec *__iodev, int __count,
@@ -136,11 +143,13 @@ extern ssize_t pwritev2 (int __fd, const struct iovec *__iodev, int __count,
 extern ssize_t __REDIRECT (pwritev2, (int __fd, const struct iovec *__iovec,
 				      int __count, __off64_t __offset,
 				      int __flags),
-			   pwritev64v2) __wur;
+			   pwritev64v2)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 extern ssize_t __REDIRECT (preadv2, (int __fd, const struct iovec *__iovec,
 				     int __count, __off64_t __offset,
 				     int __flags),
-			   preadv64v2) __wur;
+			   preadv64v2)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 #  else
 #   define preadv2 preadv64v2
 #   define pwritev2 pwritev64v2
@@ -151,12 +160,14 @@ extern ssize_t __REDIRECT (preadv2, (int __fd, const struct iovec *__iovec,
 /* Same as preadv but with an additional flag argumenti defined at uio.h.  */
 extern ssize_t preadv64v2 (int __fp, const struct iovec *__iovec,
 			   int __count, __off64_t __offset,
-			   int ___flags) __wur;
+			   int ___flags)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 
 /* Same as preadv but with an additional flag argument defined at uio.h.  */
 extern ssize_t pwritev64v2 (int __fd, const struct iovec *__iodev,
 			    int __count, __off64_t __offset,
-			    int __flags) __wur;
+			    int __flags)
+  __wur __attr_access ((__read_only__, 2, 3)) __nonnull ((2));
 # endif
 #endif /* Use GNU.  */
 
diff --git a/posix/regex.h b/posix/regex.h
index 5fe41c8685..75c9201fc6 100644
--- a/posix/regex.h
+++ b/posix/regex.h
@@ -536,7 +536,8 @@ extern reg_syntax_t re_set_syntax (reg_syntax_t __syntax);
    'regcomp', with a malloc'ed value, or set to NULL before calling
    'regfree'.  */
 extern const char *re_compile_pattern (const char *__pattern, size_t __length,
-				       struct re_pattern_buffer *__buffer);
+				       struct re_pattern_buffer *__buffer)
+    __attr_access ((__read_only__, 1, 2));
 
 
 /* Compile a fastmap for the compiled pattern in BUFFER; used to
@@ -553,7 +554,8 @@ extern int re_compile_fastmap (struct re_pattern_buffer *__buffer);
 extern regoff_t re_search (struct re_pattern_buffer *__buffer,
 			   const char *__String, regoff_t __length,
 			   regoff_t __start, regoff_t __range,
-			   struct re_registers *__regs);
+			   struct re_registers *__regs)
+    __attr_access ((__read_only__, 2, 3));
 
 
 /* Like 're_search', but search in the concatenation of STRING1 and
@@ -563,14 +565,17 @@ extern regoff_t re_search_2 (struct re_pattern_buffer *__buffer,
 			     const char *__string2, regoff_t __length2,
 			     regoff_t __start, regoff_t __range,
 			     struct re_registers *__regs,
-			     regoff_t __stop);
+			     regoff_t __stop)
+    __attr_access ((__read_only__, 2, 3))
+    __attr_access ((__read_only__, 4, 5));
 
 
 /* Like 're_search', but return how many characters in STRING the regexp
    in BUFFER matched, starting at position START.  */
 extern regoff_t re_match (struct re_pattern_buffer *__buffer,
 			  const char *__String, regoff_t __length,
-			  regoff_t __start, struct re_registers *__regs);
+			  regoff_t __start, struct re_registers *__regs)
+    __attr_access ((__read_only__, 2, 3));
 
 
 /* Relates to 're_match' as 're_search_2' relates to 're_search'.  */
@@ -578,7 +583,9 @@ extern regoff_t re_match_2 (struct re_pattern_buffer *__buffer,
 			    const char *__string1, regoff_t __length1,
 			    const char *__string2, regoff_t __length2,
 			    regoff_t __start, struct re_registers *__regs,
-			    regoff_t __stop);
+			    regoff_t __stop)
+    __attr_access ((__read_only__, 2, 3))
+    __attr_access ((__read_only__, 4, 5));
 
 
 /* Set REGS to hold NUM_REGS registers, storing them in STARTS and
@@ -641,10 +648,12 @@ extern int regcomp (regex_t *_Restrict_ __preg,
 extern int regexec (const regex_t *_Restrict_ __preg,
 		    const char *_Restrict_ __String, size_t __nmatch,
 		    regmatch_t __pmatch[_Restrict_arr_],
-		    int __eflags);
+		    int __eflags)
+    __attr_access ((__write_only__, 4, 3));
 
 extern size_t regerror (int __errcode, const regex_t *_Restrict_ __preg,
-			char *_Restrict_ __errbuf, size_t __errbuf_size);
+			char *_Restrict_ __errbuf, size_t __errbuf_size)
+    __attr_access ((__write_only__, 3, 4));
 
 extern void regfree (regex_t *__preg);
 
diff --git a/posix/unistd.h b/posix/unistd.h
index 32b8161619..0eb79fb6ce 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -1156,10 +1156,12 @@ extern void swab (const void *__restrict __from, void *__restrict __to,
    <stdio.h>.  */
 #if defined __USE_XOPEN && !defined __USE_XOPEN2K
 /* Return the name of the controlling terminal.  */
-extern char *ctermid (char *__s) __THROW;
+extern char *ctermid (char __s[_L_ctermid]) __THROW
+    __attr_access ((__write_only__, 1));
 
 /* Return the name of the current user.  */
-extern char *cuserid (char *__s);
+extern char *cuserid (char __s[_L_cuserid])
+    __attr_access ((__write_only__, 1))
 #endif
 
 
diff --git a/pwd/pwd.h b/pwd/pwd.h
index bbc29479cd..065a19bade 100644
--- a/pwd/pwd.h
+++ b/pwd/pwd.h
@@ -139,20 +139,23 @@ extern struct passwd *getpwnam (const char *__name) __nonnull ((1));
 extern int getpwent_r (struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((1, 2, 4));
+    __nonnull ((1, 2, 4))
+    __attr_access ((__write_only__, 2, 3));
 # endif
 
 extern int getpwuid_r (__uid_t __uid,
 		       struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((2, 3, 5));
+    __nonnull ((2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 
 extern int getpwnam_r (const char *__restrict __name,
 		       struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((1, 2, 3, 5));
+    __nonnull ((1, 2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 
 
 # ifdef	__USE_MISC
@@ -167,7 +170,8 @@ extern int fgetpwent_r (FILE *__restrict __stream,
 			struct passwd *__restrict __resultbuf,
 			char *__restrict __buffer, size_t __buflen,
 			struct passwd **__restrict __result)
-			__nonnull ((1, 2, 3, 5));
+    __nonnull ((1, 2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 # endif
 
 #endif	/* POSIX or reentrant */
diff --git a/stdio-common/cuserid.c b/stdio-common/cuserid.c
index d4f1861c99..578887e82b 100644
--- a/stdio-common/cuserid.c
+++ b/stdio-common/cuserid.c
@@ -22,7 +22,7 @@
    If S is not NULL, it points to a buffer of at least L_cuserid bytes
    into which the name is copied; otherwise, a static buffer is used.  */
 char *
-cuserid (char *s)
+cuserid (char s[L_cuserid])
 {
   __set_errno (ENOSYS);
   return NULL;
diff --git a/stdio-common/stdio_lim.h.in b/stdio-common/stdio_lim.h.in
index de0caa5fee..9e3c59c942 100644
--- a/stdio-common/stdio_lim.h.in
+++ b/stdio-common/stdio_lim.h.in
@@ -26,6 +26,9 @@
 #define TMP_MAX @TMP_MAX@
 #define FILENAME_MAX @FILENAME_MAX@
 
+#define _L_ctermid @L_ctermid@
+#define _L_cuserid @L_cuserid@
+
 #ifdef __USE_POSIX
 # define L_ctermid @L_ctermid@
 # if !defined __USE_XOPEN2K || defined __USE_GNU
diff --git a/stdio-common/tmpnam.c b/stdio-common/tmpnam.c
index a8e0ca5b3b..cdd64e1d54 100644
--- a/stdio-common/tmpnam.c
+++ b/stdio-common/tmpnam.c
@@ -24,7 +24,7 @@ static char tmpnam_buffer[L_tmpnam];
 
    This function is *not* thread safe!  */
 char *
-tmpnam (char *s)
+tmpnam (char s[L_tmpnam])
 {
   /* By using two buffers we manage to be thread safe in the case
      where S != NULL.  */
diff --git a/stdio-common/tmpnam_r.c b/stdio-common/tmpnam_r.c
index 49f762f392..e359e4808a 100644
--- a/stdio-common/tmpnam_r.c
+++ b/stdio-common/tmpnam_r.c
@@ -20,7 +20,7 @@
 /* Generate a unique filename in P_tmpdir.  If S is NULL return NULL.
    This makes this function thread safe.  */
 char *
-tmpnam_r (char *s)
+tmpnam_r (char s[L_tmpnam])
 {
   if (s == NULL)
     return NULL;
diff --git a/stdlib/monetary.h b/stdlib/monetary.h
index c9d3c64e14..37ee8ab6d2 100644
--- a/stdlib/monetary.h
+++ b/stdlib/monetary.h
@@ -37,7 +37,8 @@ __BEGIN_DECLS
 /* Formatting a monetary value according to the current locale.  */
 extern ssize_t strfmon (char *__restrict __s, size_t __maxsize,
 			const char *__restrict __format, ...)
-     __THROW __attribute_format_strfmon__ (3, 4);
+     __THROW __attribute_format_strfmon__ (3, 4)
+     __attr_access ((__write_only__, 1, 2));
 
 #ifdef __USE_XOPEN2K8
 /* POSIX.1-2008 extended locale interface (see locale.h).  */
@@ -47,7 +48,8 @@ extern ssize_t strfmon (char *__restrict __s, size_t __maxsize,
 extern ssize_t strfmon_l (char *__restrict __s, size_t __maxsize,
 			  locale_t __loc,
 			  const char *__restrict __format, ...)
-     __THROW __attribute_format_strfmon__ (4, 5);
+     __THROW __attribute_format_strfmon__ (4, 5)
+     __attr_access ((__write_only__, 1, 2));
 #endif
 
 #include <bits/floatn.h>
diff --git a/sysdeps/gnu/net/if.h b/sysdeps/gnu/net/if.h
index e587a5ce59..30f3e281e5 100644
--- a/sysdeps/gnu/net/if.h
+++ b/sysdeps/gnu/net/if.h
@@ -191,7 +191,9 @@ __BEGIN_DECLS
 
 /* Convert an interface name to an index, and vice versa.  */
 extern unsigned int if_nametoindex (const char *__ifname) __THROW;
-extern char *if_indextoname (unsigned int __ifindex, char *__ifname) __THROW;
+extern char *if_indextoname (unsigned int __ifindex,
+			     char __ifname[IF_NAMESIZE]) __THROW
+    __attr_access ((__write_only__, 2));
 
 /* Return a list of all interfaces and their indices.  */
 extern struct if_nameindex *if_nameindex (void) __THROW;
diff --git a/sysdeps/mach/hurd/if_index.c b/sysdeps/mach/hurd/if_index.c
index 32dceccdbf..f92cd5723a 100644
--- a/sysdeps/mach/hurd/if_index.c
+++ b/sysdeps/mach/hurd/if_index.c
@@ -166,7 +166,7 @@ libc_hidden_weak (if_nameindex)
    IFNAME (which has space for at least IFNAMSIZ characters).  Return
    IFNAME, or NULL on error.  */
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   struct ifreq ifr;
   int fd = __opensock ();
diff --git a/sysdeps/posix/ctermid.c b/sysdeps/posix/ctermid.c
index 49f3f08a6f..4112cb0f2f 100644
--- a/sysdeps/posix/ctermid.c
+++ b/sysdeps/posix/ctermid.c
@@ -24,7 +24,7 @@
    long), otherwise we return a pointer to a non-const but read-only
    string literal, that POSIX states the caller must not modify.  */
 char *
-ctermid (char *s)
+ctermid (char s[L_ctermid])
 {
   char *name = (char /*drop const*/ *) "/dev/tty";
 
diff --git a/sysdeps/posix/cuserid.c b/sysdeps/posix/cuserid.c
index 401b100333..213802f0f9 100644
--- a/sysdeps/posix/cuserid.c
+++ b/sysdeps/posix/cuserid.c
@@ -25,7 +25,7 @@
    If S is not NULL, it points to a buffer of at least L_cuserid bytes
    into which the name is copied; otherwise, a static buffer is used.  */
 char *
-cuserid (char *s)
+cuserid (char s[L_cuserid])
 {
   static char name[L_cuserid];
   char buf[NSS_BUFLEN_PASSWD];
diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c
index dffa363931..5509da2c21 100644
--- a/sysdeps/unix/sysv/linux/if_index.c
+++ b/sysdeps/unix/sysv/linux/if_index.c
@@ -215,7 +215,7 @@ libc_hidden_weak (if_nameindex)
 
 
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   /* We may be able to do the conversion directly, rather than searching a
      list.  This ioctl is not present in kernels before version 2.1.50.  */

Re: [PATCH] more out of bounds checking improvements

[Florian Weimer via Libc-alpha]

* Martin Sebor via Libc-alpha:

> Besides attribute access, the change adds attribute nonnull to
> the readv and writev functions in misc/sys/uio.h.  The functions
> don't necessarily access the array elements when their count is
> zero but neither POSIX nor the Linux manual document this so it
> seems appropriate to warn.

This change is questionable because it breaks interoperability with
abstract data types such as std::vector, where the base pointer can be
null when the array is empty.  The kernel does not return EFAULT in this
case, as expected.

I think we need a special case for this.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

On 10/26/20 9:41 AM, Florian Weimer wrote:
> * Martin Sebor via Libc-alpha:
> 
>> Besides attribute access, the change adds attribute nonnull to
>> the readv and writev functions in misc/sys/uio.h.  The functions
>> don't necessarily access the array elements when their count is
>> zero but neither POSIX nor the Linux manual document this so it
>> seems appropriate to warn.
> 
> This change is questionable because it breaks interoperability with
> abstract data types such as std::vector, where the base pointer can be
> null when the array is empty.  The kernel does not return EFAULT in this
> case, as expected.

Let me separate this out from the main patch if it isn't
straightforward.  Although POSIX doesn't require the functions to
fail, it does suggest, albeit obliquely, they may fail when the iov
pointer is null:

   The iovcnt argument is valid if greater than 0 and less than or
   equal to {IOV_MAX}, as defined in <limits.h>.

   The writev() function may fail and set errno to:

   [EINVAL]
     The iovcnt argument was less than or equal to 0, or greater than
     {IOV_MAX}.

Passing the functions a null iov pointer is only realistic when
iovcnt is zero.  The former is nonconforming and undefined under
any conditions (null isn't a valid argument to any library
function unless specified otherwise), and the latter may cause
the function to fail.  A warning for it seems to me both
appropriate and helpful, certainly in the absence of the Glibc
or Linux man pages documenting the behavior under these
conditions.

Martin

> I think we need a special case for this.
> 
> Thanks,
> Florian
> 

Re: [PATCH] more out of bounds checking improvements

[Joseph Myers]

On Mon, 26 Oct 2020, Martin Sebor via Libc-alpha wrote:

> The patch introduces the _L_tmpnam macro to avoid polluting
> the POSIX <unistd.h> namespace with L_tmpnam when the latter is
> only supposed to be defined in <stdio.h>.  This in turn causes
> the a number of POSIX conformance test failures that I haven't
> been able to figure how to deal with and need some help with.
> 
> In file included from ../include/unistd.h:2,
>                  from /tmp/tmpzm39v4n3/test.c:1:
> ../posix/unistd.h:1159:32: error: ‘_L_ctermid’ undeclared here (not in a
> function)
>  extern char *ctermid (char __s[_L_ctermid]) __THROW
>                                 ^~~~~~~~~~
> 
> I expected adding the new macros to stdio-common/stdio_lim.h.in
> would do the trick but clearly something else is needed and I'm
> at a lost as to what that might be.  I haven't been able to find

<unistd.h> doesn't include <bits/stdio_lim.h>, and you're making 
<unistd.h> use _L_ctermid, and you're only defining _L_ctermid in 
<bits/stdio_lim.h>.  You need to define it in a header that <unistd.h> 
includes - which also needs to be one whose contents are namespace-clean 
for inclusion in <unistd.h> (which <bits/stdio_lim.h> isn't).

The obvious way would be to have a new installed (i.e. add to "headers" in 
the relevant Makefile) header for the new macros that can be included in 
both <stdio.h> and <unistd.h>.  Suggestion: the existing scheme for 
automatic generation of bits/stdio_lim.h is overly complicated, it would 
be better to use sysdeps headers in the normal way like for other bits/ 
headers where the values may depend on the glibc configuration (and then 
to have testcases that verify consistently of OPEN_MAX and FOPEN_MAX / of 
PATH_MAX and FILENAME_MAX, when both are defined).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 2383 bytes --]

On 10/26/20 10:08 AM, Joseph Myers wrote:
> On Mon, 26 Oct 2020, Martin Sebor via Libc-alpha wrote:
> 
>> The patch introduces the _L_tmpnam macro to avoid polluting
>> the POSIX <unistd.h> namespace with L_tmpnam when the latter is
>> only supposed to be defined in <stdio.h>.  This in turn causes
>> the a number of POSIX conformance test failures that I haven't
>> been able to figure how to deal with and need some help with.
>>
>> In file included from ../include/unistd.h:2,
>>                   from /tmp/tmpzm39v4n3/test.c:1:
>> ../posix/unistd.h:1159:32: error: ‘_L_ctermid’ undeclared here (not in a
>> function)
>>   extern char *ctermid (char __s[_L_ctermid]) __THROW
>>                                  ^~~~~~~~~~
>>
>> I expected adding the new macros to stdio-common/stdio_lim.h.in
>> would do the trick but clearly something else is needed and I'm
>> at a lost as to what that might be.  I haven't been able to find
> 
> <unistd.h> doesn't include <bits/stdio_lim.h>, and you're making
> <unistd.h> use _L_ctermid, and you're only defining _L_ctermid in
> <bits/stdio_lim.h>.  You need to define it in a header that <unistd.h>
> includes - which also needs to be one whose contents are namespace-clean
> for inclusion in <unistd.h> (which <bits/stdio_lim.h> isn't).
> 
> The obvious way would be to have a new installed (i.e. add to "headers" in
> the relevant Makefile) header for the new macros that can be included in
> both <stdio.h> and <unistd.h>.  Suggestion: the existing scheme for
> automatic generation of bits/stdio_lim.h is overly complicated, it would
> be better to use sysdeps headers in the normal way like for other bits/
> headers where the values may depend on the glibc configuration (and then
> to have testcases that verify consistently of OPEN_MAX and FOPEN_MAX / of
> PATH_MAX and FILENAME_MAX, when both are defined).

I don't know enough about the Glibc build infrastructure to
understand your suggestion but either approach sounds more involved
than I have cycles for so I propose the scaled patch instead, without
the ctermid and cuserid changes (and without the nonnull attribute
on readv/writev(*)).  Hopefully someone with more experience with
the existing scheme will find a way to define the two macros and
make use of them to enable the detection for these two functions
as well.

Martin

[*] I'll submit that patch separately.

[-- Attachment #2: glibc-attr-access.diff --]
[-- Type: text/x-patch, Size: 17104 bytes --]

diff --git a/inet/if_index.c b/inet/if_index.c
index ddef419230..36f1806ac1 100644
--- a/inet/if_index.c
+++ b/inet/if_index.c
@@ -31,7 +31,7 @@ libc_hidden_weak (if_nametoindex)
 stub_warning (if_nametoindex)
 
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   __set_errno (ENOSYS);
   return NULL;
diff --git a/io/bits/poll2.h b/io/bits/poll2.h
index dca49717db..8cbedb7542 100644
--- a/io/bits/poll2.h
+++ b/io/bits/poll2.h
@@ -26,13 +26,14 @@ __BEGIN_DECLS
 extern int __REDIRECT (__poll_alias, (struct pollfd *__fds, nfds_t __nfds,
 				      int __timeout), poll);
 extern int __poll_chk (struct pollfd *__fds, nfds_t __nfds, int __timeout,
-		       __SIZE_TYPE__ __fdslen);
+		       __SIZE_TYPE__ __fdslen)
+    __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT (__poll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 					 int __timeout, __SIZE_TYPE__ __fdslen),
 		       __poll_chk)
   __warnattr ("poll called with fds buffer too small file nfds entries");
 
-__fortify_function int
+__fortify_function __attr_access ((__write_only__, 1, 2)) int
 poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
 {
   if (__bos (__fds) != (__SIZE_TYPE__) -1)
@@ -53,7 +54,8 @@ extern int __REDIRECT (__ppoll_alias, (struct pollfd *__fds, nfds_t __nfds,
 				       const __sigset_t *__ss), ppoll);
 extern int __ppoll_chk (struct pollfd *__fds, nfds_t __nfds,
 			const struct timespec *__timeout,
-			const __sigset_t *__ss, __SIZE_TYPE__ __fdslen);
+			const __sigset_t *__ss, __SIZE_TYPE__ __fdslen)
+    __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT (__ppoll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 					  const struct timespec *__timeout,
 					  const __sigset_t *__ss,
@@ -61,7 +63,7 @@ extern int __REDIRECT (__ppoll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 		       __ppoll_chk)
   __warnattr ("ppoll called with fds buffer too small file nfds entries");
 
-__fortify_function int
+__fortify_function __attr_access ((__write_only__, 1, 2)) int
 ppoll (struct pollfd *__fds, nfds_t __nfds, const struct timespec *__timeout,
        const __sigset_t *__ss)
 {
diff --git a/io/sys/poll.h b/io/sys/poll.h
index 857be0f5ac..aa145a1737 100644
--- a/io/sys/poll.h
+++ b/io/sys/poll.h
@@ -51,7 +51,8 @@ __BEGIN_DECLS
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout);
+extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
+    __attr_access ((__write_only__, 1, 2));
 
 #ifdef __USE_GNU
 /* Like poll, but before waiting the threads signal mask is replaced
@@ -62,7 +63,9 @@ extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout);
    __THROW.  */
 extern int ppoll (struct pollfd *__fds, nfds_t __nfds,
 		  const struct timespec *__timeout,
-		  const __sigset_t *__ss);
+		  const __sigset_t *__ss)
+    __attr_access ((__write_only__, 1, 2));
+
 #endif
 
 __END_DECLS
diff --git a/libio/stdio.h b/libio/stdio.h
index 998470943e..fb44f1e40f 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -184,12 +184,12 @@ extern FILE *tmpfile64 (void) __wur;
 #endif
 
 /* Generate a temporary filename.  */
-extern char *tmpnam (char *__s) __THROW __wur;
+extern char *tmpnam (char[L_tmpnam]) __THROW __wur;
 
 #ifdef __USE_MISC
 /* This is the reentrant variant of `tmpnam'.  The only difference is
    that it does not allow S to be NULL.  */
-extern char *tmpnam_r (char *__s) __THROW __wur;
+extern char *tmpnam_r (char __s[L_tmpnam]) __THROW __wur;
 #endif
 
 
@@ -808,13 +808,15 @@ extern int pclose (FILE *__stream);
 
 #ifdef	__USE_POSIX
 /* Return the name of the controlling terminal.  */
-extern char *ctermid (char *__s) __THROW;
+extern char *ctermid (char *__s) __THROW
+  __attr_access ((__write_only__, 1));
 #endif /* Use POSIX.  */
 
 
 #if (defined __USE_XOPEN && !defined __USE_XOPEN2K) || defined __USE_GNU
 /* Return the name of the current user.  */
-extern char *cuserid (char *__s);
+extern char *cuserid (char *__s)
+  __attr_access ((__write_only__, 1));
 #endif /* Use X/Open, but not issue 6.  */
 
 
diff --git a/misc/sys/uio.h b/misc/sys/uio.h
index 26d87c9f34..2e09858adc 100644
--- a/misc/sys/uio.h
+++ b/misc/sys/uio.h
@@ -39,7 +39,7 @@ __BEGIN_DECLS
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t readv (int __fd, const struct iovec *__iovec, int __count)
-  __wur;
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Write data pointed by the buffers described by IOVEC, which
    is a vector of COUNT 'struct iovec's, to file descriptor FD.
@@ -50,7 +50,7 @@ extern ssize_t readv (int __fd, const struct iovec *__iovec, int __count)
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
-  __wur;
+  __wur __attr_access ((__read_only__, 2, 3));
 
 
 #ifdef __USE_MISC
@@ -65,7 +65,8 @@ extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count,
-		       __off_t __offset) __wur;
+		       __off_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Write data pointed by the buffers described by IOVEC, which is a
    vector of COUNT 'struct iovec's, to file descriptor FD at the given
@@ -77,16 +78,19 @@ extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pwritev (int __fd, const struct iovec *__iovec, int __count,
-			__off_t __offset) __wur;
+			__off_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 # else
 #  ifdef __REDIRECT
 extern ssize_t __REDIRECT (preadv, (int __fd, const struct iovec *__iovec,
 				    int __count, __off64_t __offset),
-			   preadv64) __wur;
+			   preadv64)
+  __wur __attr_access ((__read_only__, 2, 3));
 extern ssize_t __REDIRECT (pwritev, (int __fd, const struct iovec *__iovec,
 				     int __count, __off64_t __offset),
-			   pwritev64) __wur;
+			   pwritev64)
+  __wur __attr_access ((__read_only__, 2, 3));
 #  else
 #   define preadv preadv64
 #   define pwritev pwritev64
@@ -104,7 +108,8 @@ extern ssize_t __REDIRECT (pwritev, (int __fd, const struct iovec *__iovec,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t preadv64 (int __fd, const struct iovec *__iovec, int __count,
-			 __off64_t __offset) __wur;
+			 __off64_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Write data pointed by the buffers described by IOVEC, which is a
    vector of COUNT 'struct iovec's, to file descriptor FD at the given
@@ -116,7 +121,8 @@ extern ssize_t preadv64 (int __fd, const struct iovec *__iovec, int __count,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pwritev64 (int __fd, const struct iovec *__iovec, int __count,
-			  __off64_t __offset) __wur;
+			  __off64_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3));
 # endif
 #endif	/* Use misc.  */
 
@@ -125,7 +131,8 @@ extern ssize_t pwritev64 (int __fd, const struct iovec *__iovec, int __count,
 # ifndef __USE_FILE_OFFSET64
 /* Same as preadv but with an additional flag argumenti defined at uio.h.  */
 extern ssize_t preadv2 (int __fp, const struct iovec *__iovec, int __count,
-			__off_t __offset, int ___flags) __wur;
+			__off_t __offset, int ___flags)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Same as preadv but with an additional flag argument defined at uio.h.  */
 extern ssize_t pwritev2 (int __fd, const struct iovec *__iodev, int __count,
@@ -136,11 +143,13 @@ extern ssize_t pwritev2 (int __fd, const struct iovec *__iodev, int __count,
 extern ssize_t __REDIRECT (pwritev2, (int __fd, const struct iovec *__iovec,
 				      int __count, __off64_t __offset,
 				      int __flags),
-			   pwritev64v2) __wur;
+			   pwritev64v2)
+  __wur __attr_access ((__read_only__, 2, 3));
 extern ssize_t __REDIRECT (preadv2, (int __fd, const struct iovec *__iovec,
 				     int __count, __off64_t __offset,
 				     int __flags),
-			   preadv64v2) __wur;
+			   preadv64v2)
+  __wur __attr_access ((__read_only__, 2, 3));
 #  else
 #   define preadv2 preadv64v2
 #   define pwritev2 pwritev64v2
@@ -151,12 +160,14 @@ extern ssize_t __REDIRECT (preadv2, (int __fd, const struct iovec *__iovec,
 /* Same as preadv but with an additional flag argumenti defined at uio.h.  */
 extern ssize_t preadv64v2 (int __fp, const struct iovec *__iovec,
 			   int __count, __off64_t __offset,
-			   int ___flags) __wur;
+			   int ___flags)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Same as preadv but with an additional flag argument defined at uio.h.  */
 extern ssize_t pwritev64v2 (int __fd, const struct iovec *__iodev,
 			    int __count, __off64_t __offset,
-			    int __flags) __wur;
+			    int __flags)
+  __wur __attr_access ((__read_only__, 2, 3));
 # endif
 #endif /* Use GNU.  */
 
diff --git a/posix/regex.h b/posix/regex.h
index 5fe41c8685..75c9201fc6 100644
--- a/posix/regex.h
+++ b/posix/regex.h
@@ -536,7 +536,8 @@ extern reg_syntax_t re_set_syntax (reg_syntax_t __syntax);
    'regcomp', with a malloc'ed value, or set to NULL before calling
    'regfree'.  */
 extern const char *re_compile_pattern (const char *__pattern, size_t __length,
-				       struct re_pattern_buffer *__buffer);
+				       struct re_pattern_buffer *__buffer)
+    __attr_access ((__read_only__, 1, 2));
 
 
 /* Compile a fastmap for the compiled pattern in BUFFER; used to
@@ -553,7 +554,8 @@ extern int re_compile_fastmap (struct re_pattern_buffer *__buffer);
 extern regoff_t re_search (struct re_pattern_buffer *__buffer,
 			   const char *__String, regoff_t __length,
 			   regoff_t __start, regoff_t __range,
-			   struct re_registers *__regs);
+			   struct re_registers *__regs)
+    __attr_access ((__read_only__, 2, 3));
 
 
 /* Like 're_search', but search in the concatenation of STRING1 and
@@ -563,14 +565,17 @@ extern regoff_t re_search_2 (struct re_pattern_buffer *__buffer,
 			     const char *__string2, regoff_t __length2,
 			     regoff_t __start, regoff_t __range,
 			     struct re_registers *__regs,
-			     regoff_t __stop);
+			     regoff_t __stop)
+    __attr_access ((__read_only__, 2, 3))
+    __attr_access ((__read_only__, 4, 5));
 
 
 /* Like 're_search', but return how many characters in STRING the regexp
    in BUFFER matched, starting at position START.  */
 extern regoff_t re_match (struct re_pattern_buffer *__buffer,
 			  const char *__String, regoff_t __length,
-			  regoff_t __start, struct re_registers *__regs);
+			  regoff_t __start, struct re_registers *__regs)
+    __attr_access ((__read_only__, 2, 3));
 
 
 /* Relates to 're_match' as 're_search_2' relates to 're_search'.  */
@@ -578,7 +583,9 @@ extern regoff_t re_match_2 (struct re_pattern_buffer *__buffer,
 			    const char *__string1, regoff_t __length1,
 			    const char *__string2, regoff_t __length2,
 			    regoff_t __start, struct re_registers *__regs,
-			    regoff_t __stop);
+			    regoff_t __stop)
+    __attr_access ((__read_only__, 2, 3))
+    __attr_access ((__read_only__, 4, 5));
 
 
 /* Set REGS to hold NUM_REGS registers, storing them in STARTS and
@@ -641,10 +648,12 @@ extern int regcomp (regex_t *_Restrict_ __preg,
 extern int regexec (const regex_t *_Restrict_ __preg,
 		    const char *_Restrict_ __String, size_t __nmatch,
 		    regmatch_t __pmatch[_Restrict_arr_],
-		    int __eflags);
+		    int __eflags)
+    __attr_access ((__write_only__, 4, 3));
 
 extern size_t regerror (int __errcode, const regex_t *_Restrict_ __preg,
-			char *_Restrict_ __errbuf, size_t __errbuf_size);
+			char *_Restrict_ __errbuf, size_t __errbuf_size)
+    __attr_access ((__write_only__, 3, 4));
 
 extern void regfree (regex_t *__preg);
 
diff --git a/pwd/pwd.h b/pwd/pwd.h
index bbc29479cd..065a19bade 100644
--- a/pwd/pwd.h
+++ b/pwd/pwd.h
@@ -139,20 +139,23 @@ extern struct passwd *getpwnam (const char *__name) __nonnull ((1));
 extern int getpwent_r (struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((1, 2, 4));
+    __nonnull ((1, 2, 4))
+    __attr_access ((__write_only__, 2, 3));
 # endif
 
 extern int getpwuid_r (__uid_t __uid,
 		       struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((2, 3, 5));
+    __nonnull ((2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 
 extern int getpwnam_r (const char *__restrict __name,
 		       struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((1, 2, 3, 5));
+    __nonnull ((1, 2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 
 
 # ifdef	__USE_MISC
@@ -167,7 +170,8 @@ extern int fgetpwent_r (FILE *__restrict __stream,
 			struct passwd *__restrict __resultbuf,
 			char *__restrict __buffer, size_t __buflen,
 			struct passwd **__restrict __result)
-			__nonnull ((1, 2, 3, 5));
+    __nonnull ((1, 2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 # endif
 
 #endif	/* POSIX or reentrant */
diff --git a/stdio-common/tmpnam.c b/stdio-common/tmpnam.c
index a8e0ca5b3b..cdd64e1d54 100644
--- a/stdio-common/tmpnam.c
+++ b/stdio-common/tmpnam.c
@@ -24,7 +24,7 @@ static char tmpnam_buffer[L_tmpnam];
 
    This function is *not* thread safe!  */
 char *
-tmpnam (char *s)
+tmpnam (char s[L_tmpnam])
 {
   /* By using two buffers we manage to be thread safe in the case
      where S != NULL.  */
diff --git a/stdio-common/tmpnam_r.c b/stdio-common/tmpnam_r.c
index 49f762f392..e359e4808a 100644
--- a/stdio-common/tmpnam_r.c
+++ b/stdio-common/tmpnam_r.c
@@ -20,7 +20,7 @@
 /* Generate a unique filename in P_tmpdir.  If S is NULL return NULL.
    This makes this function thread safe.  */
 char *
-tmpnam_r (char *s)
+tmpnam_r (char s[L_tmpnam])
 {
   if (s == NULL)
     return NULL;
diff --git a/stdlib/monetary.h b/stdlib/monetary.h
index c9d3c64e14..37ee8ab6d2 100644
--- a/stdlib/monetary.h
+++ b/stdlib/monetary.h
@@ -37,7 +37,8 @@ __BEGIN_DECLS
 /* Formatting a monetary value according to the current locale.  */
 extern ssize_t strfmon (char *__restrict __s, size_t __maxsize,
 			const char *__restrict __format, ...)
-     __THROW __attribute_format_strfmon__ (3, 4);
+     __THROW __attribute_format_strfmon__ (3, 4)
+     __attr_access ((__write_only__, 1, 2));
 
 #ifdef __USE_XOPEN2K8
 /* POSIX.1-2008 extended locale interface (see locale.h).  */
@@ -47,7 +48,8 @@ extern ssize_t strfmon (char *__restrict __s, size_t __maxsize,
 extern ssize_t strfmon_l (char *__restrict __s, size_t __maxsize,
 			  locale_t __loc,
 			  const char *__restrict __format, ...)
-     __THROW __attribute_format_strfmon__ (4, 5);
+     __THROW __attribute_format_strfmon__ (4, 5)
+     __attr_access ((__write_only__, 1, 2));
 #endif
 
 #include <bits/floatn.h>
diff --git a/sysdeps/gnu/net/if.h b/sysdeps/gnu/net/if.h
index e587a5ce59..30f3e281e5 100644
--- a/sysdeps/gnu/net/if.h
+++ b/sysdeps/gnu/net/if.h
@@ -191,7 +191,9 @@ __BEGIN_DECLS
 
 /* Convert an interface name to an index, and vice versa.  */
 extern unsigned int if_nametoindex (const char *__ifname) __THROW;
-extern char *if_indextoname (unsigned int __ifindex, char *__ifname) __THROW;
+extern char *if_indextoname (unsigned int __ifindex,
+			     char __ifname[IF_NAMESIZE]) __THROW
+    __attr_access ((__write_only__, 2));
 
 /* Return a list of all interfaces and their indices.  */
 extern struct if_nameindex *if_nameindex (void) __THROW;
diff --git a/sysdeps/mach/hurd/if_index.c b/sysdeps/mach/hurd/if_index.c
index 32dceccdbf..f92cd5723a 100644
--- a/sysdeps/mach/hurd/if_index.c
+++ b/sysdeps/mach/hurd/if_index.c
@@ -166,7 +166,7 @@ libc_hidden_weak (if_nameindex)
    IFNAME (which has space for at least IFNAMSIZ characters).  Return
    IFNAME, or NULL on error.  */
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   struct ifreq ifr;
   int fd = __opensock ();
diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c
index dffa363931..5509da2c21 100644
--- a/sysdeps/unix/sysv/linux/if_index.c
+++ b/sysdeps/unix/sysv/linux/if_index.c
@@ -215,7 +215,7 @@ libc_hidden_weak (if_nameindex)
 
 
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   /* We may be able to do the conversion directly, rather than searching a
      list.  This ioctl is not present in kernels before version 2.1.50.  */

Ping: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

Ping: Does the last patch look good enough to commit?
https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html

On 12/9/20 2:46 PM, Martin Sebor wrote:
> On 10/26/20 10:08 AM, Joseph Myers wrote:
>> On Mon, 26 Oct 2020, Martin Sebor via Libc-alpha wrote:
>>
>>> The patch introduces the _L_tmpnam macro to avoid polluting
>>> the POSIX <unistd.h> namespace with L_tmpnam when the latter is
>>> only supposed to be defined in <stdio.h>.  This in turn causes
>>> the a number of POSIX conformance test failures that I haven't
>>> been able to figure how to deal with and need some help with.
>>>
>>> In file included from ../include/unistd.h:2,
>>>                   from /tmp/tmpzm39v4n3/test.c:1:
>>> ../posix/unistd.h:1159:32: error: ‘_L_ctermid’ undeclared here (not in a
>>> function)
>>>   extern char *ctermid (char __s[_L_ctermid]) __THROW
>>>                                  ^~~~~~~~~~
>>>
>>> I expected adding the new macros to stdio-common/stdio_lim.h.in
>>> would do the trick but clearly something else is needed and I'm
>>> at a lost as to what that might be.  I haven't been able to find
>>
>> <unistd.h> doesn't include <bits/stdio_lim.h>, and you're making
>> <unistd.h> use _L_ctermid, and you're only defining _L_ctermid in
>> <bits/stdio_lim.h>.  You need to define it in a header that <unistd.h>
>> includes - which also needs to be one whose contents are namespace-clean
>> for inclusion in <unistd.h> (which <bits/stdio_lim.h> isn't).
>>
>> The obvious way would be to have a new installed (i.e. add to 
>> "headers" in
>> the relevant Makefile) header for the new macros that can be included in
>> both <stdio.h> and <unistd.h>.  Suggestion: the existing scheme for
>> automatic generation of bits/stdio_lim.h is overly complicated, it would
>> be better to use sysdeps headers in the normal way like for other bits/
>> headers where the values may depend on the glibc configuration (and then
>> to have testcases that verify consistently of OPEN_MAX and FOPEN_MAX / of
>> PATH_MAX and FILENAME_MAX, when both are defined).
> 
> I don't know enough about the Glibc build infrastructure to
> understand your suggestion but either approach sounds more involved
> than I have cycles for so I propose the scaled patch instead, without
> the ctermid and cuserid changes (and without the nonnull attribute
> on readv/writev(*)).  Hopefully someone with more experience with
> the existing scheme will find a way to define the two macros and
> make use of them to enable the detection for these two functions
> as well.
> 
> Martin
> 
> [*] I'll submit that patch separately.

Ping 2: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

Joseph or anyone else: is the patch below okay to commit?  I'd like
to include it in the upcoming release.

https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html

On 12/18/20 9:56 AM, Martin Sebor wrote:
> Ping: Does the last patch look good enough to commit?
> https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html
> 
> On 12/9/20 2:46 PM, Martin Sebor wrote:
>> On 10/26/20 10:08 AM, Joseph Myers wrote:
>>> On Mon, 26 Oct 2020, Martin Sebor via Libc-alpha wrote:
>>>
>>>> The patch introduces the _L_tmpnam macro to avoid polluting
>>>> the POSIX <unistd.h> namespace with L_tmpnam when the latter is
>>>> only supposed to be defined in <stdio.h>.  This in turn causes
>>>> the a number of POSIX conformance test failures that I haven't
>>>> been able to figure how to deal with and need some help with.
>>>>
>>>> In file included from ../include/unistd.h:2,
>>>>                   from /tmp/tmpzm39v4n3/test.c:1:
>>>> ../posix/unistd.h:1159:32: error: ‘_L_ctermid’ undeclared here (not 
>>>> in a
>>>> function)
>>>>   extern char *ctermid (char __s[_L_ctermid]) __THROW
>>>>                                  ^~~~~~~~~~
>>>>
>>>> I expected adding the new macros to stdio-common/stdio_lim.h.in
>>>> would do the trick but clearly something else is needed and I'm
>>>> at a lost as to what that might be.  I haven't been able to find
>>>
>>> <unistd.h> doesn't include <bits/stdio_lim.h>, and you're making
>>> <unistd.h> use _L_ctermid, and you're only defining _L_ctermid in
>>> <bits/stdio_lim.h>.  You need to define it in a header that <unistd.h>
>>> includes - which also needs to be one whose contents are namespace-clean
>>> for inclusion in <unistd.h> (which <bits/stdio_lim.h> isn't).
>>>
>>> The obvious way would be to have a new installed (i.e. add to 
>>> "headers" in
>>> the relevant Makefile) header for the new macros that can be included in
>>> both <stdio.h> and <unistd.h>.  Suggestion: the existing scheme for
>>> automatic generation of bits/stdio_lim.h is overly complicated, it would
>>> be better to use sysdeps headers in the normal way like for other bits/
>>> headers where the values may depend on the glibc configuration (and then
>>> to have testcases that verify consistently of OPEN_MAX and FOPEN_MAX 
>>> / of
>>> PATH_MAX and FILENAME_MAX, when both are defined).
>>
>> I don't know enough about the Glibc build infrastructure to
>> understand your suggestion but either approach sounds more involved
>> than I have cycles for so I propose the scaled patch instead, without
>> the ctermid and cuserid changes (and without the nonnull attribute
>> on readv/writev(*)).  Hopefully someone with more experience with
>> the existing scheme will find a way to define the two macros and
>> make use of them to enable the detection for these two functions
>> as well.
>>
>> Martin
>>
>> [*] I'll submit that patch separately.
> 

Ping 3: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

Ping: still looking for an approval of the patch below before
tomorrow's freeze:
https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html

On 1/4/21 8:54 AM, Martin Sebor wrote:
> Joseph or anyone else: is the patch below okay to commit?  I'd like
> to include it in the upcoming release.
> 
> https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html
> 
> On 12/18/20 9:56 AM, Martin Sebor wrote:
>> Ping: Does the last patch look good enough to commit?
>> https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html
>>
>> On 12/9/20 2:46 PM, Martin Sebor wrote:
>>> On 10/26/20 10:08 AM, Joseph Myers wrote:
>>>> On Mon, 26 Oct 2020, Martin Sebor via Libc-alpha wrote:
>>>>
>>>>> The patch introduces the _L_tmpnam macro to avoid polluting
>>>>> the POSIX <unistd.h> namespace with L_tmpnam when the latter is
>>>>> only supposed to be defined in <stdio.h>.  This in turn causes
>>>>> the a number of POSIX conformance test failures that I haven't
>>>>> been able to figure how to deal with and need some help with.
>>>>>
>>>>> In file included from ../include/unistd.h:2,
>>>>>                   from /tmp/tmpzm39v4n3/test.c:1:
>>>>> ../posix/unistd.h:1159:32: error: ‘_L_ctermid’ undeclared here (not 
>>>>> in a
>>>>> function)
>>>>>   extern char *ctermid (char __s[_L_ctermid]) __THROW
>>>>>                                  ^~~~~~~~~~
>>>>>
>>>>> I expected adding the new macros to stdio-common/stdio_lim.h.in
>>>>> would do the trick but clearly something else is needed and I'm
>>>>> at a lost as to what that might be.  I haven't been able to find
>>>>
>>>> <unistd.h> doesn't include <bits/stdio_lim.h>, and you're making
>>>> <unistd.h> use _L_ctermid, and you're only defining _L_ctermid in
>>>> <bits/stdio_lim.h>.  You need to define it in a header that <unistd.h>
>>>> includes - which also needs to be one whose contents are 
>>>> namespace-clean
>>>> for inclusion in <unistd.h> (which <bits/stdio_lim.h> isn't).
>>>>
>>>> The obvious way would be to have a new installed (i.e. add to 
>>>> "headers" in
>>>> the relevant Makefile) header for the new macros that can be 
>>>> included in
>>>> both <stdio.h> and <unistd.h>.  Suggestion: the existing scheme for
>>>> automatic generation of bits/stdio_lim.h is overly complicated, it 
>>>> would
>>>> be better to use sysdeps headers in the normal way like for other bits/
>>>> headers where the values may depend on the glibc configuration (and 
>>>> then
>>>> to have testcases that verify consistently of OPEN_MAX and FOPEN_MAX 
>>>> / of
>>>> PATH_MAX and FILENAME_MAX, when both are defined).
>>>
>>> I don't know enough about the Glibc build infrastructure to
>>> understand your suggestion but either approach sounds more involved
>>> than I have cycles for so I propose the scaled patch instead, without
>>> the ctermid and cuserid changes (and without the nonnull attribute
>>> on readv/writev(*)).  Hopefully someone with more experience with
>>> the existing scheme will find a way to define the two macros and
>>> make use of them to enable the detection for these two functions
>>> as well.
>>>
>>> Martin
>>>
>>> [*] I'll submit that patch separately.
>>
> 

Ping 4: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

Florian reminded me that this patch (first posted on 10/26 last
year) is still outstanding so I'd like to try to ping it again.

Florian and Joseph provided some initial comments but not a formal
approval.

Martin

On 1/10/21 1:44 PM, Martin Sebor wrote:
> Ping: still looking for an approval of the patch below before
> tomorrow's freeze:
> https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html
> 
> On 1/4/21 8:54 AM, Martin Sebor wrote:
>> Joseph or anyone else: is the patch below okay to commit?  I'd like
>> to include it in the upcoming release.
>>
>> https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html
>>
>> On 12/18/20 9:56 AM, Martin Sebor wrote:
>>> Ping: Does the last patch look good enough to commit?
>>> https://sourceware.org/pipermail/libc-alpha/2020-December/120586.html
>>>
>>> On 12/9/20 2:46 PM, Martin Sebor wrote:
>>>> On 10/26/20 10:08 AM, Joseph Myers wrote:
>>>>> On Mon, 26 Oct 2020, Martin Sebor via Libc-alpha wrote:
>>>>>
>>>>>> The patch introduces the _L_tmpnam macro to avoid polluting
>>>>>> the POSIX <unistd.h> namespace with L_tmpnam when the latter is
>>>>>> only supposed to be defined in <stdio.h>.  This in turn causes
>>>>>> the a number of POSIX conformance test failures that I haven't
>>>>>> been able to figure how to deal with and need some help with.
>>>>>>
>>>>>> In file included from ../include/unistd.h:2,
>>>>>>                   from /tmp/tmpzm39v4n3/test.c:1:
>>>>>> ../posix/unistd.h:1159:32: error: ‘_L_ctermid’ undeclared here 
>>>>>> (not in a
>>>>>> function)
>>>>>>   extern char *ctermid (char __s[_L_ctermid]) __THROW
>>>>>>                                  ^~~~~~~~~~
>>>>>>
>>>>>> I expected adding the new macros to stdio-common/stdio_lim.h.in
>>>>>> would do the trick but clearly something else is needed and I'm
>>>>>> at a lost as to what that might be.  I haven't been able to find
>>>>>
>>>>> <unistd.h> doesn't include <bits/stdio_lim.h>, and you're making
>>>>> <unistd.h> use _L_ctermid, and you're only defining _L_ctermid in
>>>>> <bits/stdio_lim.h>.  You need to define it in a header that <unistd.h>
>>>>> includes - which also needs to be one whose contents are 
>>>>> namespace-clean
>>>>> for inclusion in <unistd.h> (which <bits/stdio_lim.h> isn't).
>>>>>
>>>>> The obvious way would be to have a new installed (i.e. add to 
>>>>> "headers" in
>>>>> the relevant Makefile) header for the new macros that can be 
>>>>> included in
>>>>> both <stdio.h> and <unistd.h>.  Suggestion: the existing scheme for
>>>>> automatic generation of bits/stdio_lim.h is overly complicated, it 
>>>>> would
>>>>> be better to use sysdeps headers in the normal way like for other 
>>>>> bits/
>>>>> headers where the values may depend on the glibc configuration (and 
>>>>> then
>>>>> to have testcases that verify consistently of OPEN_MAX and 
>>>>> FOPEN_MAX / of
>>>>> PATH_MAX and FILENAME_MAX, when both are defined).
>>>>
>>>> I don't know enough about the Glibc build infrastructure to
>>>> understand your suggestion but either approach sounds more involved
>>>> than I have cycles for so I propose the scaled patch instead, without
>>>> the ctermid and cuserid changes (and without the nonnull attribute
>>>> on readv/writev(*)).  Hopefully someone with more experience with
>>>> the existing scheme will find a way to define the two macros and
>>>> make use of them to enable the detection for these two functions
>>>> as well.
>>>>
>>>> Martin
>>>>
>>>> [*] I'll submit that patch separately.
>>>
>>
> 

Re: [PATCH] more out of bounds checking improvements

[Florian Weimer via Libc-alpha]

* Martin Sebor via Libc-alpha:

> diff --git a/sysdeps/gnu/net/if.h b/sysdeps/gnu/net/if.h
> index e587a5ce59..30f3e281e5 100644
> --- a/sysdeps/gnu/net/if.h
> +++ b/sysdeps/gnu/net/if.h
> @@ -191,7 +191,9 @@ __BEGIN_DECLS
>  
>  /* Convert an interface name to an index, and vice versa.  */
>  extern unsigned int if_nametoindex (const char *__ifname) __THROW;
> -extern char *if_indextoname (unsigned int __ifindex, char *__ifname) __THROW;
> +extern char *if_indextoname (unsigned int __ifindex,
> +			     char __ifname[IF_NAMESIZE]) __THROW
> +    __attr_access ((__write_only__, 2));
>  
>  /* Return a list of all interfaces and their indices.  */
>  extern struct if_nameindex *if_nameindex (void) __THROW;

Is the change from a pointer to an array allowed by POSIX?

Thanks,
Florian

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

On 4/23/21 4:31 AM, Florian Weimer wrote:
> * Martin Sebor via Libc-alpha:
> 
>> diff --git a/sysdeps/gnu/net/if.h b/sysdeps/gnu/net/if.h
>> index e587a5ce59..30f3e281e5 100644
>> --- a/sysdeps/gnu/net/if.h
>> +++ b/sysdeps/gnu/net/if.h
>> @@ -191,7 +191,9 @@ __BEGIN_DECLS
>>   
>>   /* Convert an interface name to an index, and vice versa.  */
>>   extern unsigned int if_nametoindex (const char *__ifname) __THROW;
>> -extern char *if_indextoname (unsigned int __ifindex, char *__ifname) __THROW;
>> +extern char *if_indextoname (unsigned int __ifindex,
>> +			     char __ifname[IF_NAMESIZE]) __THROW
>> +    __attr_access ((__write_only__, 2));
>>   
>>   /* Return a list of all interfaces and their indices.  */
>>   extern struct if_nameindex *if_nameindex (void) __THROW;
> 
> Is the change from a pointer to an array allowed by POSIX?

There's no way for a conforming program to tell how a function pointer
parameter is declared so I believe it is.

Martin

Re: [PATCH] more out of bounds checking improvements

[Florian Weimer via Libc-alpha]

* Martin Sebor:

> On 4/23/21 4:31 AM, Florian Weimer wrote:
>> * Martin Sebor via Libc-alpha:
>> 
>>> diff --git a/sysdeps/gnu/net/if.h b/sysdeps/gnu/net/if.h
>>> index e587a5ce59..30f3e281e5 100644
>>> --- a/sysdeps/gnu/net/if.h
>>> +++ b/sysdeps/gnu/net/if.h
>>> @@ -191,7 +191,9 @@ __BEGIN_DECLS
>>>     /* Convert an interface name to an index, and vice versa.  */
>>>   extern unsigned int if_nametoindex (const char *__ifname) __THROW;
>>> -extern char *if_indextoname (unsigned int __ifindex, char *__ifname) __THROW;
>>> +extern char *if_indextoname (unsigned int __ifindex,
>>> +			     char __ifname[IF_NAMESIZE]) __THROW
>>> +    __attr_access ((__write_only__, 2));
>>>     /* Return a list of all interfaces and their indices.  */
>>>   extern struct if_nameindex *if_nameindex (void) __THROW;
>> Is the change from a pointer to an array allowed by POSIX?
>
> There's no way for a conforming program to tell how a function pointer
> parameter is declared so I believe it is.

Hmm, I think you might be right.  It's like parameter names in this
regard.

Thanks,
Florian

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

On 4/23/21 10:01 AM, Florian Weimer wrote:
> * Martin Sebor:
> 
>> On 4/23/21 4:31 AM, Florian Weimer wrote:
>>> * Martin Sebor via Libc-alpha:
>>>
>>>> diff --git a/sysdeps/gnu/net/if.h b/sysdeps/gnu/net/if.h
>>>> index e587a5ce59..30f3e281e5 100644
>>>> --- a/sysdeps/gnu/net/if.h
>>>> +++ b/sysdeps/gnu/net/if.h
>>>> @@ -191,7 +191,9 @@ __BEGIN_DECLS
>>>>      /* Convert an interface name to an index, and vice versa.  */
>>>>    extern unsigned int if_nametoindex (const char *__ifname) __THROW;
>>>> -extern char *if_indextoname (unsigned int __ifindex, char *__ifname) __THROW;
>>>> +extern char *if_indextoname (unsigned int __ifindex,
>>>> +			     char __ifname[IF_NAMESIZE]) __THROW
>>>> +    __attr_access ((__write_only__, 2));
>>>>      /* Return a list of all interfaces and their indices.  */
>>>>    extern struct if_nameindex *if_nameindex (void) __THROW;
>>> Is the change from a pointer to an array allowed by POSIX?
>>
>> There's no way for a conforming program to tell how a function pointer
>> parameter is declared so I believe it is.
> 
> Hmm, I think you might be right.  It's like parameter names in this
> regard.

You sound disappointed ;)

If there are no further comments/concerns I'll retest the patch on top
of the latest trunk and if all goes well go ahead with the commit later
this week.

Martin

> 
> Thanks,
> Florian
> 

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

On 5/4/21 1:58 PM, Martin Sebor wrote:
> On 4/23/21 10:01 AM, Florian Weimer wrote:
>> * Martin Sebor:
>>
>>> On 4/23/21 4:31 AM, Florian Weimer wrote:
>>>> * Martin Sebor via Libc-alpha:
>>>>
>>>>> diff --git a/sysdeps/gnu/net/if.h b/sysdeps/gnu/net/if.h
>>>>> index e587a5ce59..30f3e281e5 100644
>>>>> --- a/sysdeps/gnu/net/if.h
>>>>> +++ b/sysdeps/gnu/net/if.h
>>>>> @@ -191,7 +191,9 @@ __BEGIN_DECLS
>>>>>      /* Convert an interface name to an index, and vice versa.  */
>>>>>    extern unsigned int if_nametoindex (const char *__ifname) __THROW;
>>>>> -extern char *if_indextoname (unsigned int __ifindex, char 
>>>>> *__ifname) __THROW;
>>>>> +extern char *if_indextoname (unsigned int __ifindex,
>>>>> +                 char __ifname[IF_NAMESIZE]) __THROW
>>>>> +    __attr_access ((__write_only__, 2));
>>>>>      /* Return a list of all interfaces and their indices.  */
>>>>>    extern struct if_nameindex *if_nameindex (void) __THROW;
>>>> Is the change from a pointer to an array allowed by POSIX?
>>>
>>> There's no way for a conforming program to tell how a function pointer
>>> parameter is declared so I believe it is.
>>
>> Hmm, I think you might be right.  It's like parameter names in this
>> regard.
> 
> You sound disappointed ;)
> 
> If there are no further comments/concerns I'll retest the patch on top
> of the latest trunk and if all goes well go ahead with the commit later
> this week.

Retesting the changes with GCC 7 through 11 exposed a few minor bugs
in Glibc tests and a possible false positive for makedb.c with GCC 10.
I have fixed those and committed the attached patch in g:26492c0a14.

Martin

[-- Attachment #2: glibc-attr-access.diff --]
[-- Type: text/x-patch, Size: 20605 bytes --]

commit 26492c0a14966c32c43cd6ca1d0dca5e62c6cfef
Author: Martin Sebor <msebor@redhat.com>
Date:   Thu May 6 10:56:25 2021 -0600

    Annotate additional APIs with GCC attribute access.
    
    This change continues the improvements to compile-time out of bounds
    checking by decorating more APIs with either attribute access, or by
    explicitly providing the array bound in APIs such as tmpnam() that
    expect arrays of some minimum size as arguments.  (The latter feature
    is new in GCC 11.)
    
    The only effects of the attribute and/or the array bound is to check
    and diagnose calls to the functions that fail to provide a sufficient
    number of elements, and the definitions of the functions that access
    elements outside the specified bounds.  (There is no interplay with
    _FORTIFY_SOURCE here yet.)
    
    Tested with GCC 7 through 11 on x86_64-linux.

diff --git a/inet/if_index.c b/inet/if_index.c
index b85a93f9a4..0442633142 100644
--- a/inet/if_index.c
+++ b/inet/if_index.c
@@ -31,7 +31,7 @@ libc_hidden_weak (if_nametoindex)
 stub_warning (if_nametoindex)
 
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   __set_errno (ENOSYS);
   return NULL;
diff --git a/io/bits/poll2.h b/io/bits/poll2.h
index 882fcc9ea2..a623678c09 100644
--- a/io/bits/poll2.h
+++ b/io/bits/poll2.h
@@ -26,13 +26,14 @@ __BEGIN_DECLS
 extern int __REDIRECT (__poll_alias, (struct pollfd *__fds, nfds_t __nfds,
 				      int __timeout), poll);
 extern int __poll_chk (struct pollfd *__fds, nfds_t __nfds, int __timeout,
-		       __SIZE_TYPE__ __fdslen);
+		       __SIZE_TYPE__ __fdslen)
+    __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT (__poll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 					 int __timeout, __SIZE_TYPE__ __fdslen),
 		       __poll_chk)
   __warnattr ("poll called with fds buffer too small file nfds entries");
 
-__fortify_function int
+__fortify_function __attr_access ((__write_only__, 1, 2)) int
 poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
 {
   if (__glibc_objsize (__fds) != (__SIZE_TYPE__) -1)
@@ -54,7 +55,8 @@ extern int __REDIRECT (__ppoll_alias, (struct pollfd *__fds, nfds_t __nfds,
 				       const __sigset_t *__ss), ppoll);
 extern int __ppoll_chk (struct pollfd *__fds, nfds_t __nfds,
 			const struct timespec *__timeout,
-			const __sigset_t *__ss, __SIZE_TYPE__ __fdslen);
+			const __sigset_t *__ss, __SIZE_TYPE__ __fdslen)
+    __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT (__ppoll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 					  const struct timespec *__timeout,
 					  const __sigset_t *__ss,
@@ -62,7 +64,7 @@ extern int __REDIRECT (__ppoll_chk_warn, (struct pollfd *__fds, nfds_t __nfds,
 		       __ppoll_chk)
   __warnattr ("ppoll called with fds buffer too small file nfds entries");
 
-__fortify_function int
+__fortify_function __attr_access ((__write_only__, 1, 2)) int
 ppoll (struct pollfd *__fds, nfds_t __nfds, const struct timespec *__timeout,
        const __sigset_t *__ss)
 {
diff --git a/io/sys/poll.h b/io/sys/poll.h
index 2431dd1e14..08f29df540 100644
--- a/io/sys/poll.h
+++ b/io/sys/poll.h
@@ -51,7 +51,8 @@ __BEGIN_DECLS
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout);
+extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout)
+    __attr_access ((__write_only__, 1, 2));
 
 #ifdef __USE_GNU
 /* Like poll, but before waiting the threads signal mask is replaced
@@ -62,7 +63,9 @@ extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout);
    __THROW.  */
 extern int ppoll (struct pollfd *__fds, nfds_t __nfds,
 		  const struct timespec *__timeout,
-		  const __sigset_t *__ss);
+		  const __sigset_t *__ss)
+    __attr_access ((__write_only__, 1, 2));
+
 #endif
 
 __END_DECLS
diff --git a/libio/stdio.h b/libio/stdio.h
index 144137cf67..76bda3728e 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -184,12 +184,12 @@ extern FILE *tmpfile64 (void) __wur;
 #endif
 
 /* Generate a temporary filename.  */
-extern char *tmpnam (char *__s) __THROW __wur;
+extern char *tmpnam (char[L_tmpnam]) __THROW __wur;
 
 #ifdef __USE_MISC
 /* This is the reentrant variant of `tmpnam'.  The only difference is
    that it does not allow S to be NULL.  */
-extern char *tmpnam_r (char *__s) __THROW __wur;
+extern char *tmpnam_r (char __s[L_tmpnam]) __THROW __wur;
 #endif
 
 
@@ -808,13 +808,15 @@ extern int pclose (FILE *__stream);
 
 #ifdef	__USE_POSIX
 /* Return the name of the controlling terminal.  */
-extern char *ctermid (char *__s) __THROW;
+extern char *ctermid (char *__s) __THROW
+  __attr_access ((__write_only__, 1));
 #endif /* Use POSIX.  */
 
 
 #if (defined __USE_XOPEN && !defined __USE_XOPEN2K) || defined __USE_GNU
 /* Return the name of the current user.  */
-extern char *cuserid (char *__s);
+extern char *cuserid (char *__s)
+  __attr_access ((__write_only__, 1));
 #endif /* Use X/Open, but not issue 6.  */
 
 
diff --git a/misc/sys/uio.h b/misc/sys/uio.h
index 474527d112..08dcb39c99 100644
--- a/misc/sys/uio.h
+++ b/misc/sys/uio.h
@@ -39,7 +39,7 @@ __BEGIN_DECLS
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t readv (int __fd, const struct iovec *__iovec, int __count)
-  __wur;
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Write data pointed by the buffers described by IOVEC, which
    is a vector of COUNT 'struct iovec's, to file descriptor FD.
@@ -50,7 +50,7 @@ extern ssize_t readv (int __fd, const struct iovec *__iovec, int __count)
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
-  __wur;
+  __wur __attr_access ((__read_only__, 2, 3));
 
 
 #ifdef __USE_MISC
@@ -65,7 +65,8 @@ extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count,
-		       __off_t __offset) __wur;
+		       __off_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Write data pointed by the buffers described by IOVEC, which is a
    vector of COUNT 'struct iovec's, to file descriptor FD at the given
@@ -77,16 +78,19 @@ extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pwritev (int __fd, const struct iovec *__iovec, int __count,
-			__off_t __offset) __wur;
+			__off_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 # else
 #  ifdef __REDIRECT
 extern ssize_t __REDIRECT (preadv, (int __fd, const struct iovec *__iovec,
 				    int __count, __off64_t __offset),
-			   preadv64) __wur;
+			   preadv64)
+  __wur __attr_access ((__read_only__, 2, 3));
 extern ssize_t __REDIRECT (pwritev, (int __fd, const struct iovec *__iovec,
 				     int __count, __off64_t __offset),
-			   pwritev64) __wur;
+			   pwritev64)
+  __wur __attr_access ((__read_only__, 2, 3));
 #  else
 #   define preadv preadv64
 #   define pwritev pwritev64
@@ -104,7 +108,8 @@ extern ssize_t __REDIRECT (pwritev, (int __fd, const struct iovec *__iovec,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t preadv64 (int __fd, const struct iovec *__iovec, int __count,
-			 __off64_t __offset) __wur;
+			 __off64_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Write data pointed by the buffers described by IOVEC, which is a
    vector of COUNT 'struct iovec's, to file descriptor FD at the given
@@ -116,7 +121,8 @@ extern ssize_t preadv64 (int __fd, const struct iovec *__iovec, int __count,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pwritev64 (int __fd, const struct iovec *__iovec, int __count,
-			  __off64_t __offset) __wur;
+			  __off64_t __offset)
+  __wur __attr_access ((__read_only__, 2, 3));
 # endif
 #endif	/* Use misc.  */
 
@@ -125,7 +131,8 @@ extern ssize_t pwritev64 (int __fd, const struct iovec *__iovec, int __count,
 # ifndef __USE_FILE_OFFSET64
 /* Same as preadv but with an additional flag argumenti defined at uio.h.  */
 extern ssize_t preadv2 (int __fp, const struct iovec *__iovec, int __count,
-			__off_t __offset, int ___flags) __wur;
+			__off_t __offset, int ___flags)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Same as preadv but with an additional flag argument defined at uio.h.  */
 extern ssize_t pwritev2 (int __fd, const struct iovec *__iodev, int __count,
@@ -136,11 +143,13 @@ extern ssize_t pwritev2 (int __fd, const struct iovec *__iodev, int __count,
 extern ssize_t __REDIRECT (pwritev2, (int __fd, const struct iovec *__iovec,
 				      int __count, __off64_t __offset,
 				      int __flags),
-			   pwritev64v2) __wur;
+			   pwritev64v2)
+  __wur __attr_access ((__read_only__, 2, 3));
 extern ssize_t __REDIRECT (preadv2, (int __fd, const struct iovec *__iovec,
 				     int __count, __off64_t __offset,
 				     int __flags),
-			   preadv64v2) __wur;
+			   preadv64v2)
+  __wur __attr_access ((__read_only__, 2, 3));
 #  else
 #   define preadv2 preadv64v2
 #   define pwritev2 pwritev64v2
@@ -151,12 +160,14 @@ extern ssize_t __REDIRECT (preadv2, (int __fd, const struct iovec *__iovec,
 /* Same as preadv but with an additional flag argumenti defined at uio.h.  */
 extern ssize_t preadv64v2 (int __fp, const struct iovec *__iovec,
 			   int __count, __off64_t __offset,
-			   int ___flags) __wur;
+			   int ___flags)
+  __wur __attr_access ((__read_only__, 2, 3));
 
 /* Same as preadv but with an additional flag argument defined at uio.h.  */
 extern ssize_t pwritev64v2 (int __fd, const struct iovec *__iodev,
 			    int __count, __off64_t __offset,
-			    int __flags) __wur;
+			    int __flags)
+  __wur __attr_access ((__read_only__, 2, 3));
 # endif
 #endif /* Use GNU.  */
 
diff --git a/nss/makedb.c b/nss/makedb.c
index 74edb749cf..9389f6b548 100644
--- a/nss/makedb.c
+++ b/nss/makedb.c
@@ -747,7 +747,8 @@ write_output (int fd)
   header->valstrlen = valstrlen;
 
   size_t filled_dbs = 0;
-  struct iovec iov[2 + ndatabases * 3];
+  size_t iov_nelts = 2 + ndatabases * 3;
+  struct iovec iov[iov_nelts];
   iov[0].iov_base = header;
   iov[0].iov_len = file_offset;
 
@@ -791,7 +792,9 @@ write_output (int fd)
 			  + nhashentries_total * sizeof (stridx_t)));
   header->allocate = file_offset;
 
-  if (writev (fd, iov, 2 + ndatabases * 3) != keydataoffset)
+  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
+  assert (iov_nelts <= INT_MAX);
+  if (writev (fd, iov, iov_nelts) != keydataoffset)
     {
       error (0, errno, gettext ("failed to write new database file"));
       return EXIT_FAILURE;
diff --git a/posix/bug-regex33.c b/posix/bug-regex33.c
index 2140cda96a..86569465cf 100644
--- a/posix/bug-regex33.c
+++ b/posix/bug-regex33.c
@@ -105,7 +105,7 @@ do_test (void)
                 /* 新処圭新, \xb7\xbd here really matches 圭,
                  * this is a reproducer of bug-regex25 */
   e = re_search (&r, "\xbf\xb7\xbd\xe8\xb7\xbd\xbf\xb7",
-                 10, 0, 10, &s);
+                 9, 0, 9, &s);
   if (e != 4)
     {
       printf ("bug-regex33.7: no match or false match: re_search() returned %d, should return 4\n", e);
diff --git a/posix/regex.h b/posix/regex.h
index 8e4ef45578..14fb1d8364 100644
--- a/posix/regex.h
+++ b/posix/regex.h
@@ -536,7 +536,8 @@ extern reg_syntax_t re_set_syntax (reg_syntax_t __syntax);
    'regcomp', with a malloc'ed value, or set to NULL before calling
    'regfree'.  */
 extern const char *re_compile_pattern (const char *__pattern, size_t __length,
-				       struct re_pattern_buffer *__buffer);
+				       struct re_pattern_buffer *__buffer)
+    __attr_access ((__read_only__, 1, 2));
 
 
 /* Compile a fastmap for the compiled pattern in BUFFER; used to
@@ -553,7 +554,8 @@ extern int re_compile_fastmap (struct re_pattern_buffer *__buffer);
 extern regoff_t re_search (struct re_pattern_buffer *__buffer,
 			   const char *__String, regoff_t __length,
 			   regoff_t __start, regoff_t __range,
-			   struct re_registers *__regs);
+			   struct re_registers *__regs)
+    __attr_access ((__read_only__, 2, 3));
 
 
 /* Like 're_search', but search in the concatenation of STRING1 and
@@ -563,14 +565,17 @@ extern regoff_t re_search_2 (struct re_pattern_buffer *__buffer,
 			     const char *__string2, regoff_t __length2,
 			     regoff_t __start, regoff_t __range,
 			     struct re_registers *__regs,
-			     regoff_t __stop);
+			     regoff_t __stop)
+    __attr_access ((__read_only__, 2, 3))
+    __attr_access ((__read_only__, 4, 5));
 
 
 /* Like 're_search', but return how many characters in STRING the regexp
    in BUFFER matched, starting at position START.  */
 extern regoff_t re_match (struct re_pattern_buffer *__buffer,
 			  const char *__String, regoff_t __length,
-			  regoff_t __start, struct re_registers *__regs);
+			  regoff_t __start, struct re_registers *__regs)
+    __attr_access ((__read_only__, 2, 3));
 
 
 /* Relates to 're_match' as 're_search_2' relates to 're_search'.  */
@@ -578,7 +583,9 @@ extern regoff_t re_match_2 (struct re_pattern_buffer *__buffer,
 			    const char *__string1, regoff_t __length1,
 			    const char *__string2, regoff_t __length2,
 			    regoff_t __start, struct re_registers *__regs,
-			    regoff_t __stop);
+			    regoff_t __stop)
+    __attr_access ((__read_only__, 2, 3))
+    __attr_access ((__read_only__, 4, 5));
 
 
 /* Set REGS to hold NUM_REGS registers, storing them in STARTS and
@@ -648,10 +655,12 @@ extern int regcomp (regex_t *_Restrict_ __preg,
 extern int regexec (const regex_t *_Restrict_ __preg,
 		    const char *_Restrict_ __String, size_t __nmatch,
 		    regmatch_t __pmatch[_Restrict_arr_],
-		    int __eflags);
+		    int __eflags)
+    __attr_access ((__write_only__, 4, 3));
 
 extern size_t regerror (int __errcode, const regex_t *_Restrict_ __preg,
-			char *_Restrict_ __errbuf, size_t __errbuf_size);
+			char *_Restrict_ __errbuf, size_t __errbuf_size)
+    __attr_access ((__write_only__, 3, 4));
 
 extern void regfree (regex_t *__preg);
 
diff --git a/pwd/pwd.h b/pwd/pwd.h
index c2ddce2b2d..ec83c0963b 100644
--- a/pwd/pwd.h
+++ b/pwd/pwd.h
@@ -139,20 +139,23 @@ extern struct passwd *getpwnam (const char *__name) __nonnull ((1));
 extern int getpwent_r (struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((1, 2, 4));
+    __nonnull ((1, 2, 4))
+    __attr_access ((__write_only__, 2, 3));
 # endif
 
 extern int getpwuid_r (__uid_t __uid,
 		       struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((2, 3, 5));
+    __nonnull ((2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 
 extern int getpwnam_r (const char *__restrict __name,
 		       struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
 		       struct passwd **__restrict __result)
-		       __nonnull ((1, 2, 3, 5));
+    __nonnull ((1, 2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 
 
 # ifdef	__USE_MISC
@@ -167,7 +170,8 @@ extern int fgetpwent_r (FILE *__restrict __stream,
 			struct passwd *__restrict __resultbuf,
 			char *__restrict __buffer, size_t __buflen,
 			struct passwd **__restrict __result)
-			__nonnull ((1, 2, 3, 5));
+    __nonnull ((1, 2, 3, 5))
+    __attr_access ((__write_only__, 3, 4));
 # endif
 
 #endif	/* POSIX or reentrant */
diff --git a/stdio-common/tmpnam.c b/stdio-common/tmpnam.c
index a5621c2aa5..701ec95606 100644
--- a/stdio-common/tmpnam.c
+++ b/stdio-common/tmpnam.c
@@ -24,7 +24,7 @@ static char tmpnam_buffer[L_tmpnam];
 
    This function is *not* thread safe!  */
 char *
-tmpnam (char *s)
+tmpnam (char s[L_tmpnam])
 {
   /* By using two buffers we manage to be thread safe in the case
      where S != NULL.  */
diff --git a/stdio-common/tmpnam_r.c b/stdio-common/tmpnam_r.c
index 3fd20308be..1af0aa82da 100644
--- a/stdio-common/tmpnam_r.c
+++ b/stdio-common/tmpnam_r.c
@@ -20,7 +20,7 @@
 /* Generate a unique filename in P_tmpdir.  If S is NULL return NULL.
    This makes this function thread safe.  */
 char *
-tmpnam_r (char *s)
+tmpnam_r (char s[L_tmpnam])
 {
   if (s == NULL)
     return NULL;
diff --git a/stdlib/monetary.h b/stdlib/monetary.h
index a29af54c0b..52a2f51002 100644
--- a/stdlib/monetary.h
+++ b/stdlib/monetary.h
@@ -37,7 +37,8 @@ __BEGIN_DECLS
 /* Formatting a monetary value according to the current locale.  */
 extern ssize_t strfmon (char *__restrict __s, size_t __maxsize,
 			const char *__restrict __format, ...)
-     __THROW __attribute_format_strfmon__ (3, 4);
+     __THROW __attribute_format_strfmon__ (3, 4)
+     __attr_access ((__write_only__, 1, 2));
 
 #ifdef __USE_XOPEN2K8
 /* POSIX.1-2008 extended locale interface (see locale.h).  */
@@ -47,7 +48,8 @@ extern ssize_t strfmon (char *__restrict __s, size_t __maxsize,
 extern ssize_t strfmon_l (char *__restrict __s, size_t __maxsize,
 			  locale_t __loc,
 			  const char *__restrict __format, ...)
-     __THROW __attribute_format_strfmon__ (4, 5);
+     __THROW __attribute_format_strfmon__ (4, 5)
+     __attr_access ((__write_only__, 1, 2));
 #endif
 
 #include <bits/floatn.h>
diff --git a/sysdeps/gnu/net/if.h b/sysdeps/gnu/net/if.h
index 61e6bc2527..79d3c88512 100644
--- a/sysdeps/gnu/net/if.h
+++ b/sysdeps/gnu/net/if.h
@@ -191,7 +191,9 @@ __BEGIN_DECLS
 
 /* Convert an interface name to an index, and vice versa.  */
 extern unsigned int if_nametoindex (const char *__ifname) __THROW;
-extern char *if_indextoname (unsigned int __ifindex, char *__ifname) __THROW;
+extern char *if_indextoname (unsigned int __ifindex,
+			     char __ifname[IF_NAMESIZE]) __THROW
+    __attr_access ((__write_only__, 2));
 
 /* Return a list of all interfaces and their indices.  */
 extern struct if_nameindex *if_nameindex (void) __THROW;
diff --git a/sysdeps/mach/hurd/if_index.c b/sysdeps/mach/hurd/if_index.c
index 56e63a4a92..0eab510453 100644
--- a/sysdeps/mach/hurd/if_index.c
+++ b/sysdeps/mach/hurd/if_index.c
@@ -166,7 +166,7 @@ libc_hidden_weak (if_nameindex)
    IFNAME (which has space for at least IFNAMSIZ characters).  Return
    IFNAME, or NULL on error.  */
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   struct ifreq ifr;
   int fd = __opensock ();
diff --git a/sysdeps/unix/sysv/linux/if_index.c b/sysdeps/unix/sysv/linux/if_index.c
index 70a16a69c4..d38340bb64 100644
--- a/sysdeps/unix/sysv/linux/if_index.c
+++ b/sysdeps/unix/sysv/linux/if_index.c
@@ -215,7 +215,7 @@ libc_hidden_weak (if_nameindex)
 
 
 char *
-__if_indextoname (unsigned int ifindex, char *ifname)
+__if_indextoname (unsigned int ifindex, char ifname[IF_NAMESIZE])
 {
   /* We may be able to do the conversion directly, rather than searching a
      list.  This ioctl is not present in kernels before version 2.1.50.  */
diff --git a/sysdeps/unix/sysv/linux/test-errno-linux.c b/sysdeps/unix/sysv/linux/test-errno-linux.c
index d63836e584..65fb90f9fc 100644
--- a/sysdeps/unix/sysv/linux/test-errno-linux.c
+++ b/sysdeps/unix/sysv/linux/test-errno-linux.c
@@ -44,6 +44,7 @@
 #include <sys/time.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <libc-diag.h>
 
 /* This is not an exhaustive test: only system calls that can be
    persuaded to fail with a consistent error code and no side effects
@@ -171,7 +172,18 @@ do_test (void)
      allocation.  */
   fails |= test_wrp2 (LIST (EINVAL, ENOMEM), mlock, (void *) -1, 1);
   fails |= test_wrp (EINVAL, nanosleep, &ts, &ts);
+
+  DIAG_POP_NEEDS_COMMENT;
+
+#if __GNUC_PREREQ (9, 0)
+  /* Suppress valid GCC warning:
+     'poll' specified size 18446744073709551608 exceeds maximum object size
+  */
+  DIAG_IGNORE_NEEDS_COMMENT (9, "-Wstringop-overflow=");
+#endif
   fails |= test_wrp (EINVAL, poll, &pollfd, -1, 0);
+  DIAG_POP_NEEDS_COMMENT;
+
   /* quotactl returns ENOSYS for kernels not configured with
      CONFIG_QUOTA, and may return EPERM if called within certain types
      of containers.  Linux 5.4 added additional argument validation

Re: [PATCH] more out of bounds checking improvements

[Joseph Myers]

On Thu, 6 May 2021, Martin Sebor via Libc-alpha wrote:

> @@ -171,7 +172,18 @@ do_test (void)
>       allocation.  */
>    fails |= test_wrp2 (LIST (EINVAL, ENOMEM), mlock, (void *) -1, 1);
>    fails |= test_wrp (EINVAL, nanosleep, &ts, &ts);
> +
> +  DIAG_POP_NEEDS_COMMENT;
> +
> +#if __GNUC_PREREQ (9, 0)
> +  /* Suppress valid GCC warning:
> +     'poll' specified size 18446744073709551608 exceeds maximum object size
> +  */
> +  DIAG_IGNORE_NEEDS_COMMENT (9, "-Wstringop-overflow=");
> +#endif
>    fails |= test_wrp (EINVAL, poll, &pollfd, -1, 0);
> +  DIAG_POP_NEEDS_COMMENT;

The first DIAG_POP_NEEDS_COMMENT looks like it should be 
DIAG_PUSH_NEEDS_COMMENT.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

On 5/6/21 12:15 PM, Joseph Myers wrote:
> On Thu, 6 May 2021, Martin Sebor via Libc-alpha wrote:
> 
>> @@ -171,7 +172,18 @@ do_test (void)
>>        allocation.  */
>>     fails |= test_wrp2 (LIST (EINVAL, ENOMEM), mlock, (void *) -1, 1);
>>     fails |= test_wrp (EINVAL, nanosleep, &ts, &ts);
>> +
>> +  DIAG_POP_NEEDS_COMMENT;
>> +
>> +#if __GNUC_PREREQ (9, 0)
>> +  /* Suppress valid GCC warning:
>> +     'poll' specified size 18446744073709551608 exceeds maximum object size
>> +  */
>> +  DIAG_IGNORE_NEEDS_COMMENT (9, "-Wstringop-overflow=");
>> +#endif
>>     fails |= test_wrp (EINVAL, poll, &pollfd, -1, 0);
>> +  DIAG_POP_NEEDS_COMMENT;
> 
> The first DIAG_POP_NEEDS_COMMENT looks like it should be
> DIAG_PUSH_NEEDS_COMMENT.
> 

Fixed!  Thanks for double-checking the final patch!

Martin

Re: [PATCH] more out of bounds checking improvements

[Andreas Schwab]

makedb.c: In function 'write_output':
makedb.c:797:7: error: 'writev' specified size 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=]
  797 |   if (writev (fd, iov, iov_nelts) != keydataoffset)
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../include/sys/uio.h:2,
                 from makedb.c:39:
../misc/sys/uio.h:52:16: note: in a call to function 'writev' declared with attribute 'read_only (2, 3)'
   52 | extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
      |                ^~~~~~
cc1: all warnings being treated as errors
make[2]: *** [../o-iterator.mk:9: /home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd/cc-base/nss/makedb.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory '/home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd/nss'
make[1]: *** [Makefile:479: nss/others] Error 2
make[1]: Leaving directory '/home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd'
make: *** [Makefile:9: all] Error 2

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] more out of bounds checking improvements

[Florian Weimer via Libc-alpha]

* Andreas Schwab:

> makedb.c: In function 'write_output':
> makedb.c:797:7: error: 'writev' specified size 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=]
>   797 |   if (writev (fd, iov, iov_nelts) != keydataoffset)
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from ../include/sys/uio.h:2,
>                  from makedb.c:39:
> ../misc/sys/uio.h:52:16: note: in a call to function 'writev' declared with attribute 'read_only (2, 3)'
>    52 | extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
>       |                ^~~~~~
> cc1: all warnings being treated as errors
> make[2]: *** [../o-iterator.mk:9: /home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd/cc-base/nss/makedb.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[2]: Leaving directory '/home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd/nss'
> make[1]: *** [Makefile:479: nss/others] Error 2
> make[1]: Leaving directory '/home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd'
> make: *** [Makefile:9: all] Error 2

Are you building with -DNDEBUG?

Yes, the assert approach will not work in that case.

Thanks,
Florian

Re: [PATCH] more out of bounds checking improvements

[Andreas Schwab]

On Mai 07 2021, Florian Weimer wrote:

> Are you building with -DNDEBUG?

No.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] more out of bounds checking improvements

[Tulio Magno Quites Machado Filho via Libc-alpha]

Andreas Schwab <schwab@linux-m68k.org> writes:

> makedb.c: In function 'write_output':
> makedb.c:797:7: error: 'writev' specified size 4294967295 exceeds maximum object size 2147483647 [-Werror=stringop-overflow=]
>   797 |   if (writev (fd, iov, iov_nelts) != keydataoffset)
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from ../include/sys/uio.h:2,
>                  from makedb.c:39:
> ../misc/sys/uio.h:52:16: note: in a call to function 'writev' declared with attribute 'read_only (2, 3)'
>    52 | extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count)
>       |                ^~~~~~
> cc1: all warnings being treated as errors
> make[2]: *** [../o-iterator.mk:9: /home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd/cc-base/nss/makedb.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[2]: Leaving directory '/home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd/nss'
> make[1]: *** [Makefile:479: nss/others] Error 2
> make[1]: Leaving directory '/home/abuild/rpmbuild/BUILD/glibc-2.33.9000.515.g3bf0b4f2cd'
> make: *** [Makefile:9: all] Error 2

I'm also reproducing this on ppc32 with GCC 10.
I'm not using -DNDEBUG.

-- 
Tulio Magno

Re: [PATCH] more out of bounds checking improvements

[Joseph Myers]

On Fri, 7 May 2021, Tulio Magno Quites Machado Filho via Libc-alpha wrote:

> I'm also reproducing this on ppc32 with GCC 10.
> I'm not using -DNDEBUG.

It looks like this appears for all 32-bit platforms with GCC 10 (but not 
GCC 11, or for 64-bit platforms).

https://sourceware.org/pipermail/libc-testresults/2021q2/007905.html

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] more out of bounds checking improvements

[Florian Weimer via Libc-alpha]

* Martin Sebor via Libc-alpha:

> diff --git a/nss/makedb.c b/nss/makedb.c
> index 74edb749cf..9389f6b548 100644
> --- a/nss/makedb.c
> +++ b/nss/makedb.c
> @@ -747,7 +747,8 @@ write_output (int fd)
>    header->valstrlen = valstrlen;
>  
>    size_t filled_dbs = 0;
> -  struct iovec iov[2 + ndatabases * 3];
> +  size_t iov_nelts = 2 + ndatabases * 3;
> +  struct iovec iov[iov_nelts];
>    iov[0].iov_base = header;
>    iov[0].iov_len = file_offset;
>  
> @@ -791,7 +792,9 @@ write_output (int fd)
>  			  + nhashentries_total * sizeof (stridx_t)));
>    header->allocate = file_offset;
>  
> -  if (writev (fd, iov, 2 + ndatabases * 3) != keydataoffset)
> +  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
> +  assert (iov_nelts <= INT_MAX);
> +  if (writev (fd, iov, iov_nelts) != keydataoffset)
>      {
>        error (0, errno, gettext ("failed to write new database file"));
>        return EXIT_FAILURE;

I don't think you should use assert to suppress compiler warnings
because we are supposed to have warning-free builds even with -DNDEBUG
(although it's likely that other problems exist).

Thanks,
Florian

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

On 5/10/21 2:45 AM, Florian Weimer wrote:
> * Martin Sebor via Libc-alpha:
> 
>> diff --git a/nss/makedb.c b/nss/makedb.c
>> index 74edb749cf..9389f6b548 100644
>> --- a/nss/makedb.c
>> +++ b/nss/makedb.c
>> @@ -747,7 +747,8 @@ write_output (int fd)
>>     header->valstrlen = valstrlen;
>>   
>>     size_t filled_dbs = 0;
>> -  struct iovec iov[2 + ndatabases * 3];
>> +  size_t iov_nelts = 2 + ndatabases * 3;
>> +  struct iovec iov[iov_nelts];
>>     iov[0].iov_base = header;
>>     iov[0].iov_len = file_offset;
>>   
>> @@ -791,7 +792,9 @@ write_output (int fd)
>>   			  + nhashentries_total * sizeof (stridx_t)));
>>     header->allocate = file_offset;
>>   
>> -  if (writev (fd, iov, 2 + ndatabases * 3) != keydataoffset)
>> +  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
>> +  assert (iov_nelts <= INT_MAX);
>> +  if (writev (fd, iov, iov_nelts) != keydataoffset)
>>       {
>>         error (0, errno, gettext ("failed to write new database file"));
>>         return EXIT_FAILURE;
> 
> I don't think you should use assert to suppress compiler warnings
> because we are supposed to have warning-free builds even with -DNDEBUG
> (although it's likely that other problems exist).

So how about the following instead of the assert?

diff --git a/nss/makedb.c b/nss/makedb.c
index 9389f6b548..6551b3cf2a 100644
--- a/nss/makedb.c
+++ b/nss/makedb.c
@@ -792,14 +792,25 @@ write_output (int fd)
                           + nhashentries_total * sizeof (stridx_t)));
    header->allocate = file_offset;

-  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
    assert (iov_nelts <= INT_MAX);
+
+#if __GNUC_PREREQ (10, 0)
+  DIAG_PUSH_NEEDS_COMMENT;
+  /* Avoid GCC 10 false positice warning: specified size exceeds maximum
+     object size.  */
+  DIAG_IGNORE_NEEDS_COMMENT (10, "-Wstringop-overflow");
+#endif
+
    if (writev (fd, iov, iov_nelts) != keydataoffset)
      {
        error (0, errno, gettext ("failed to write new database file"));
        return EXIT_FAILURE;
      }

+#if __GNUC_PREREQ (10, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
+
    return EXIT_SUCCESS;
  }

Martin

Re: [PATCH] more out of bounds checking improvements

[Florian Weimer via Libc-alpha]

* Martin Sebor:

> diff --git a/nss/makedb.c b/nss/makedb.c
> index 9389f6b548..6551b3cf2a 100644
> --- a/nss/makedb.c
> +++ b/nss/makedb.c
> @@ -792,14 +792,25 @@ write_output (int fd)
>                           + nhashentries_total * sizeof (stridx_t)));
>    header->allocate = file_offset;
>
> -  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
>    assert (iov_nelts <= INT_MAX);
> +
> +#if __GNUC_PREREQ (10, 0)
> +  DIAG_PUSH_NEEDS_COMMENT;
> +  /* Avoid GCC 10 false positice warning: specified size exceeds maximum
> +     object size.  */
> +  DIAG_IGNORE_NEEDS_COMMENT (10, "-Wstringop-overflow");
> +#endif
> +
>    if (writev (fd, iov, iov_nelts) != keydataoffset)
>      {
>        error (0, errno, gettext ("failed to write new database file"));
>        return EXIT_FAILURE;
>      }
>
> +#if __GNUC_PREREQ (10, 0)
> +  DIAG_POP_NEEDS_COMMENT;
> +#endif
> +
>    return EXIT_SUCCESS;
>  }

Typo: positice

I'm building this with build-many-glibcs.py right now, first with GCC
11, then with GCC 10.  I don't have a GCC 10 tree yet, so it's goint to
take some time, hopefully less than two hours.

If the warning happens with GCC 10 only, should we disable it for GCC 11
as well?

Thanks,
Florian

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

On 5/10/21 11:49 AM, Florian Weimer wrote:
> * Martin Sebor:
> 
>> diff --git a/nss/makedb.c b/nss/makedb.c
>> index 9389f6b548..6551b3cf2a 100644
>> --- a/nss/makedb.c
>> +++ b/nss/makedb.c
>> @@ -792,14 +792,25 @@ write_output (int fd)
>>                            + nhashentries_total * sizeof (stridx_t)));
>>     header->allocate = file_offset;
>>
>> -  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
>>     assert (iov_nelts <= INT_MAX);
>> +
>> +#if __GNUC_PREREQ (10, 0)
>> +  DIAG_PUSH_NEEDS_COMMENT;
>> +  /* Avoid GCC 10 false positice warning: specified size exceeds maximum
>> +     object size.  */
>> +  DIAG_IGNORE_NEEDS_COMMENT (10, "-Wstringop-overflow");
>> +#endif
>> +
>>     if (writev (fd, iov, iov_nelts) != keydataoffset)
>>       {
>>         error (0, errno, gettext ("failed to write new database file"));
>>         return EXIT_FAILURE;
>>       }
>>
>> +#if __GNUC_PREREQ (10, 0)
>> +  DIAG_POP_NEEDS_COMMENT;
>> +#endif
>> +
>>     return EXIT_SUCCESS;
>>   }
> 
> Typo: positice
> 
> I'm building this with build-many-glibcs.py right now, first with GCC
> 11, then with GCC 10.  I don't have a GCC 10 tree yet, so it's goint to
> take some time, hopefully less than two hours.
> 
> If the warning happens with GCC 10 only, should we disable it for GCC 11
> as well?

It doesn't need to be disabled for GCC 11 but I don't know how to
be this selective with these version macros.  If it's okay to use
__GNUC__ then this works:

index 9389f6b548..6a061e6457 100644
--- a/nss/makedb.c
+++ b/nss/makedb.c
@@ -792,7 +792,15 @@ write_output (int fd)
                           + nhashentries_total * sizeof (stridx_t)));
    header->allocate = file_offset;

-  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
+  assert (iov_nelts <= INT_MAX);
+
+#if __GNUC__ == 10
+  DIAG_PUSH_NEEDS_COMMENT;
+  /* Avoid GCC 10 false positive warning: specified size exceeds maximum
+     object size.  */
+  DIAG_IGNORE_NEEDS_COMMENT (10, "-Wstringop-overflow");
+#endif
+
    assert (iov_nelts <= INT_MAX);
    if (writev (fd, iov, iov_nelts) != keydataoffset)
      {
@@ -800,6 +808,10 @@ write_output (int fd)
        return EXIT_FAILURE;
      }

+#if __GNUC__ == 10
+  DIAG_POP_NEEDS_COMMENT;
+#endif
+
    return EXIT_SUCCESS;
  }

Martin

> 
> Thanks,
> Florian
> 

Re: [PATCH] more out of bounds checking improvements

[Andreas Schwab]

On Mai 10 2021, Martin Sebor via Libc-alpha wrote:

> It doesn't need to be disabled for GCC 11 but I don't know how to
> be this selective with these version macros.  If it's okay to use
> __GNUC__ then this works:
>
> index 9389f6b548..6a061e6457 100644
> --- a/nss/makedb.c
> +++ b/nss/makedb.c
> @@ -792,7 +792,15 @@ write_output (int fd)
>                           + nhashentries_total * sizeof (stridx_t)));
>    header->allocate = file_offset;
>
> -  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
> +  assert (iov_nelts <= INT_MAX);
> +
> +#if __GNUC__ == 10

You can use __GNUC_PREREQ (10, 0) && !__GNUC_PREREQ (11, 0)

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] more out of bounds checking improvements

[Florian Weimer via Libc-alpha]

* Andreas Schwab:

> On Mai 10 2021, Martin Sebor via Libc-alpha wrote:
>
>> It doesn't need to be disabled for GCC 11 but I don't know how to
>> be this selective with these version macros.  If it's okay to use
>> __GNUC__ then this works:
>>
>> index 9389f6b548..6a061e6457 100644
>> --- a/nss/makedb.c
>> +++ b/nss/makedb.c
>> @@ -792,7 +792,15 @@ write_output (int fd)
>>                           + nhashentries_total * sizeof (stridx_t)));
>>    header->allocate = file_offset;
>>
>> -  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
>> +  assert (iov_nelts <= INT_MAX);
>> +
>> +#if __GNUC__ == 10
>
> You can use __GNUC_PREREQ (10, 0) && !__GNUC_PREREQ (11, 0)

That works for me too.

I've tested the previous version (partially quoted above) with GCC 11
and GCC 10, and it builds.

It's too late for me to test Andreas' proposal today, but I think it
should be safe to push it.  I can retest it tomorrow with GCC 9/10/11,
just to be sure.

Thanks,
Florian

Re: [PATCH] more out of bounds checking improvements

[Martin Sebor via Libc-alpha]

On 5/10/21 1:50 PM, Florian Weimer wrote:
> * Andreas Schwab:
> 
>> On Mai 10 2021, Martin Sebor via Libc-alpha wrote:
>>
>>> It doesn't need to be disabled for GCC 11 but I don't know how to
>>> be this selective with these version macros.  If it's okay to use
>>> __GNUC__ then this works:
>>>
>>> index 9389f6b548..6a061e6457 100644
>>> --- a/nss/makedb.c
>>> +++ b/nss/makedb.c
>>> @@ -792,7 +792,15 @@ write_output (int fd)
>>>                            + nhashentries_total * sizeof (stridx_t)));
>>>     header->allocate = file_offset;
>>>
>>> -  /* Help GCC 10 see iov_nelts doesn't overflow the writev argument.  */
>>> +  assert (iov_nelts <= INT_MAX);
>>> +
>>> +#if __GNUC__ == 10
>>
>> You can use __GNUC_PREREQ (10, 0) && !__GNUC_PREREQ (11, 0)
> 
> That works for me too.
> 
> I've tested the previous version (partially quoted above) with GCC 11
> and GCC 10, and it builds.
> 
> It's too late for me to test Andreas' proposal today, but I think it
> should be safe to push it.  I can retest it tomorrow with GCC 9/10/11,
> just to be sure.

I have pushed the change.

Martin

Re: [PATCH] more out of bounds checking improvements

[Florian Weimer via Libc-alpha]

* Martin Sebor:

> I have pushed the change.

Thanks, my build tests didn't show any issues.

Florian