From owner-svn-src-all@freebsd.org Mon May 18 20:43:36 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 2BEF62DD330; Mon, 18 May 2020 20:43:36 +0000 (UTC) (envelope-from tuexen@freebsd.org) Received: from drew.franken.de (drew.ipv6.franken.de [IPv6:2001:638:a02:a001:20e:cff:fe4a:feaa]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.franken.de", Issuer "COMODO RSA Domain Validation Secure Server CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49QrZb2l5tz4Kfg; Mon, 18 May 2020 20:43:35 +0000 (UTC) (envelope-from tuexen@freebsd.org) Received: from mbp.fritz.box (ip4d1600c4.dynamic.kabel-deutschland.de [77.22.0.196]) (Authenticated sender: macmic) by mail-n.franken.de (Postfix) with ESMTPSA id A373B7220F40F; Mon, 18 May 2020 22:43:27 +0200 (CEST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) Subject: Re: svn commit: r361209 - head/sys/netinet From: Michael Tuexen In-Reply-To: Date: Mon, 18 May 2020 22:43:27 +0200 Cc: src-committers , svn-src-all , svn-src-head Content-Transfer-Encoding: quoted-printable Message-Id: References: <202005181007.04IA713t089936@repo.freebsd.org> <064C2DCD-6279-4442-A900-0ECCD50CC4FA@freebsd.org> To: cem@freebsd.org X-Mailer: Apple Mail (2.3608.80.23.2.2) X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=disabled version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on mail-n.franken.de X-Rspamd-Queue-Id: 49QrZb2l5tz4Kfg X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:680, ipnet:2001:638::/32, country:DE] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 May 2020 20:43:36 -0000 > On 18. May 2020, at 22:17, Conrad Meyer wrote: >=20 > Hi Michael, >=20 > On Mon, May 18, 2020 at 12:05 PM Michael Tuexen = wrote: >>=20 >>> On 18. May 2020, at 20:23, Conrad Meyer 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=