Date: Mon, 18 May 2020 22:43:27 +0200 From: Michael Tuexen <tuexen@freebsd.org> To: cem@freebsd.org Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r361209 - head/sys/netinet Message-ID: <ED7C6ECA-23E4-43D9-B08D-2A39027FC210@freebsd.org> In-Reply-To: <CAG6CVpVrJE7hnEeGxzeBMUEWsi_N2EtGXdH8ZbZbEQ0GtnDovw@mail.gmail.com> References: <202005181007.04IA713t089936@repo.freebsd.org> <CAG6CVpWA1gKLzacDeAFOGTKJcNrg608k5yu8CJg48_WFS0omnA@mail.gmail.com> <E8554788-B6CD-44A9-8ABF-7F3CD129F87F@freebsd.org> <CAG6CVpU3QpCr9WVsuc1eRJrEV3XuQEWGHRCZ9FHMmRmB2N67Mg@mail.gmail.com> <064C2DCD-6279-4442-A900-0ECCD50CC4FA@freebsd.org> <CAG6CVpVrJE7hnEeGxzeBMUEWsi_N2EtGXdH8ZbZbEQ0GtnDovw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 18. May 2020, at 22:17, Conrad Meyer <cem@FreeBSD.org> wrote:
>=20
> Hi Michael,
>=20
> On Mon, May 18, 2020 at 12:05 PM Michael Tuexen <tuexen@freebsd.org> =
wrote:
>>=20
>>> On 18. May 2020, at 20:23, Conrad Meyer <cem@freebsd.org> wrote:
>>=20
>>> 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
>>=20
>> 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.
>=20
> 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.
>=20
>> OK. I'll revert this change and replace it upstream by something like
>>=20
>> #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] =3D '\0';
>> }
>> #endif
>=20
> This seems like a messy solution. I'd suggest either putting
> unconditional "msg[0] =3D '\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...
>=20
>> 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.
>=20
> Sure, that's a good concern. That said,
>=20
> Glibc: =
https://github.com/bminor/glibc/blob/5f72f9800b250410cad3abfeeb09469ef12b2=
438/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=3Dvs-2019
> ("The snprintf function always stores a terminating null =
character=E2=80=A6")
> 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-L1=
01
> (always nul terminates)
> Linux (kernel):
> https://elixir.bootlin.com/linux/latest/source/lib/vsprintf.c#L2645
> (always nul terminates)
>=20
> None of these are conditional on error status.
>=20
> 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.
>=20
> Conrad
>=20
> 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=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ED7C6ECA-23E4-43D9-B08D-2A39027FC210>