svn commit: r361209 - head/sys/netinet

Michael Tuexen tuexen at freebsd.org
Mon May 18 20:43:36 UTC 2020


> On 18. May 2020, at 22:17, Conrad Meyer <cem at FreeBSD.org> wrote:
> 
> Hi Michael,
> 
> On Mon, May 18, 2020 at 12:05 PM Michael Tuexen <tuexen at freebsd.org> wrote:
>> 
>>> On 18. May 2020, at 20:23, Conrad Meyer <cem at freebsd.org> wrote:
>> 
>>> If truncation is intended, the GCC warning is spurious.  Given how
>>> often snprintf is used in this way, I wonder if it would make sense to
>>> just disable it across the entire tree.  Regardless, IMO it makes
>> 
>> The issue wasn't reported against the kernel code, but running the code
>> in userland. I don't really control the flags people are using.
> 
> Sure.  You can certainly ignore user reports corresponding to bogus
> flags, though, and encourage use of various flags.
I could, but decided to improve the situation for some people, but
wasn't realising that I made it worse for others. Sorry about that.
> 
>> OK. I'll revert this change and replace it upstream by something like
>> 
>> #if defined(__FreeBSD_)
>>        snprintf(msg, sizeof(msg), "Never sent serial number %8.8x", serial_num)
>> #else
>>        if (snprintf(msg, sizeof(msg), "Never sent serial number %8.8x", serial_num) < 0) {
>>                msg[0] = '\0';
>>        }
>> #endif
> 
> This seems like a messy solution.  I'd suggest either putting
> unconditional "msg[0] = '\0';" before snprintf() invocations, or
That would assume that in case of an error the first byte is overwitten.
> defining an snprintf wrapper function for non-FreeBSD platforms and
> using it universally.
Yeah, one can use a Macro SCTP_SNPRINTF(). Let me see...
> 
>> I don't know if other platforms guarantee that snprintf() can't fail.
>> If it fails, the stack would send out un-initialized stack memory on
>> the network.
> 
> Sure, that's a good concern.  That said,
> 
> Glibc: https://github.com/bminor/glibc/blob/5f72f9800b250410cad3abfeeb09469ef12b2438/libio/vsnprintf.c#L93-L114
> (always nul terminates)
> MS: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l?view=vs-2019
> ("The snprintf function always stores a terminating null character…")
> OpenBSD: https://github.com/openbsd/src/blob/master/lib/libc/stdio/vsnprintf.c#L60-L63
> (always nul terminates)
> NetBSD: https://github.com/NetBSD/src/blob/trunk/lib/libc/stdio/vsnprintf.c#L97-L101
> (always nul terminates)
> Linux (kernel):
> https://elixir.bootlin.com/linux/latest/source/lib/vsprintf.c#L2645
> (always nul terminates)
> 
> None of these are conditional on error status.
> 
> The only exception I found is musl libc, and in that it is a case you
> cannot encounter here (size > INT_MAX).  Arguably this is a bug in
> musl libc.  I did not dive deeper into musl and determine whether
> other errors were nul terminated or not.
> 
> Conrad
> 
> P.S., It seems dubious to be sending diagnostic formatted error
> messages out across the network.
It was and still is very helpful when debuging interop problems if you only have access
to a tracefile and can't change the running code. Like people asking you why is your
implementation sending back an ABORT when it sees this packet.

Best regards
Michael


More information about the svn-src-head mailing list