Re: [PATCH] more out of bounds checking improvements

[Martin Sebor <msebor@gmail.com>]

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