Date: Mon, 18 May 2020 13:17:30 -0700 From: Conrad Meyer <cem@freebsd.org> To: Michael Tuexen <tuexen@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: <CAG6CVpVrJE7hnEeGxzeBMUEWsi_N2EtGXdH8ZbZbEQ0GtnDovw@mail.gmail.com> In-Reply-To: <064C2DCD-6279-4442-A900-0ECCD50CC4FA@freebsd.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Michael,
On Mon, May 18, 2020 at 12:05 PM Michael Tuexen <tuexen@freebsd.org> wrote:
>
> > On 18. May 2020, at 20:23, Conrad Meyer <cem@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.
> 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", seri=
al_num)
> #else
> if (snprintf(msg, sizeof(msg), "Never sent serial number %8.8x", =
serial_num) < 0) {
> msg[0] =3D '\0';
> }
> #endif
This seems like a messy solution. I'd suggest either putting
unconditional "msg[0] =3D '\0';" before snprintf() invocations, or
defining an snprintf wrapper function for non-FreeBSD platforms and
using it universally.
> 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/5f72f9800b250410cad3abfeeb09469=
ef12b2438/libio/vsnprintf.c#L93-L114
(always nul terminates)
MS: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/snprin=
tf-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/vsnprint=
f.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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpVrJE7hnEeGxzeBMUEWsi_N2EtGXdH8ZbZbEQ0GtnDovw>