Date: Tue, 11 Feb 2020 20:38:21 +0100 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: r357761 - head/sys/netinet Message-ID: <F8AEE0BD-E803-4F80-960B-B7349A4572E5@freebsd.org> In-Reply-To: <CAG6CVpWkvf2JnD=U=dtYgBPjCF59zM40js2uGBCx=OA=EYa8xA@mail.gmail.com> References: <202002111400.01BE0R3I009898@repo.freebsd.org> <CAG6CVpWkvf2JnD=U=dtYgBPjCF59zM40js2uGBCx=OA=EYa8xA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 11. Feb 2020, at 19:18, Conrad Meyer <cem@FreeBSD.org> wrote: >=20 > Hi Michael, >=20 > On Tue, Feb 11, 2020 at 6:00 AM Michael Tuexen <tuexen@freebsd.org> = wrote: >>=20 >> Author: tuexen >> Date: Tue Feb 11 14:00:27 2020 >> New Revision: 357761 >> URL: https://svnweb.freebsd.org/changeset/base/357761 >>=20 >> Log: >> Use an int instead of a bool variable, since bool is not supported >> on all platforms the stack is running on in userland. >=20 > Maybe the platforms stuck in time before 1999 can be worked around > with something trivial like: >=20 > #if __STDC_VERSION__ < 199901L > # warn Really really consider getting a new compiler > typedef unsigned char _Bool; > # define bool _Bool > # define true ((_Bool)1) > # define false ((_Bool)0) > #endif Hi Conrad, I can revert it and get it working in a different way. However, the networking code uses ints for booleans in a lot of places. I wasn't aware that we need to use bool now. >=20 > Rather than inflicting the 80s on FreeBSD tree code? This commit > seems like a step in the wrong direction and just for example, > contravenes style(9). >=20 > SCTP is already one of the buggiest areas of the kernel, and it can't > be touched by outsiders for fear of breaking compatibility with the > purported myriad other platforms it is also ported to. By transitive > property, it also prevents modifying _any_ APIs SCTP consumes that > happen to be implemented on other platforms. This hamstrings the Which API couldn't be changed because of SCTP? I don't remember that I experienced such a discussion. I definitely never objected. > kernel for what is obviously not functional code (throw a dart at a > syzkaller report and better than 90% odds it's a SCTP panic), much > less code a reasonable person would want to use in a > security-sensitive context. I'm not arguing that there are bugs. But most of the bug only show up if you actually use SCTP. Regarding the 90% chance: Where do you get = this from: 1. Looking at http://syzkaller.backtrace.io:8080 you will find a lot of traces which involves SCTP. However, a lot of them will include = sendfile() in it's trace. If I remember it correctly, there was in the past a change to the internals of sendfile. After that change sendfile = support for SCTP seems to be broken. Whether it was working before that = change was discussed with different views (I wasn't involved since I never = worked on sendfile). Another set of panics involve calling listen and = connect on the same socket. This is as far as I know also not related to SCTP, = but to a recent change in the listen code to improve performance. All other issues I'm interested to fix. 2. Looking at https://syzkaller.appspot.com/freebsd I also see traces = which involve SCTP, but definitely not they are definitely not the = majority. 3. Looking at http://212.201.121.91:10000 I also see SCTP related panic, but also TCP related ones. >=20 > If SCTP is intended to be a port from some legacy platform, perhaps it > should live in ports rather than base? The SCTP is not a port from some other platform, but the code is also contained in a user land stack, which runs on a variety of platforms (the ones you build Browsers for). The code in the FreeBSD tree only has taken out some #ifdefed code, = which is used only on other platforms. Best regards Michael >=20 > Thanks, > Conrad
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F8AEE0BD-E803-4F80-960B-B7349A4572E5>