Re: svn commit: r357761 - head/sys/netinet

2020-02-11 Thread Michael Tuexen
> On 11. Feb 2020, at 20:52, Gleb Smirnoff  wrote:
> 
>  Michael,
> 
> On Tue, Feb 11, 2020 at 08:38:21PM +0100, Michael Tuexen wrote:
> M> I can revert it and get it working in a different way. However, the
> M> networking code uses ints for booleans in a lot of places. I wasn't
> M> aware that we need to use bool now.
> 
> We don't need to use bool, but we should use it in any new code.
> Some people may still drop ints into new code, and this can be
> tolerable, but intentionally converting from bools to ints is a
> move backwards and this isn't okay.
Ahh, OK. Thanks for the clarification.

Best regards
Michael
> 
> Thanks for redoing it with via ifdefs for legacy platforms.
> 
> -- 
> Gleb Smirnoff

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r357761 - head/sys/netinet

2020-02-11 Thread Gleb Smirnoff
  Michael,

On Tue, Feb 11, 2020 at 08:38:21PM +0100, Michael Tuexen wrote:
M> I can revert it and get it working in a different way. However, the
M> networking code uses ints for booleans in a lot of places. I wasn't
M> aware that we need to use bool now.

We don't need to use bool, but we should use it in any new code.
Some people may still drop ints into new code, and this can be
tolerable, but intentionally converting from bools to ints is a
move backwards and this isn't okay.

Thanks for redoing it with via ifdefs for legacy platforms.

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r357761 - head/sys/netinet

2020-02-11 Thread Michael Tuexen
> On 11. Feb 2020, at 19:18, Conrad Meyer  wrote:
> 
> Hi Michael,
> 
> On Tue, Feb 11, 2020 at 6:00 AM Michael Tuexen  wrote:
>> 
>> Author: tuexen
>> Date: Tue Feb 11 14:00:27 2020
>> New Revision: 357761
>> URL: https://svnweb.freebsd.org/changeset/base/357761
>> 
>> Log:
>>  Use an int instead of a bool variable, since bool is not supported
>>  on all platforms the stack is running on in userland.
> 
> Maybe the platforms stuck in time before 1999 can be worked around
> with something trivial like:
> 
> #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.
> 
> 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).
> 
> 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:1 I also see SCTP related panic,
   but also TCP related ones.
> 
> 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
> 
> Thanks,
> Conrad

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r357761 - head/sys/netinet

2020-02-11 Thread Conrad Meyer
Hi Michael,

On Tue, Feb 11, 2020 at 6:00 AM Michael Tuexen  wrote:
>
> Author: tuexen
> Date: Tue Feb 11 14:00:27 2020
> New Revision: 357761
> URL: https://svnweb.freebsd.org/changeset/base/357761
>
> Log:
>   Use an int instead of a bool variable, since bool is not supported
>   on all platforms the stack is running on in userland.

Maybe the platforms stuck in time before 1999 can be worked around
with something trivial like:

#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

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).

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
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.

If SCTP is intended to be a port from some legacy platform, perhaps it
should live in ports rather than base?

Thanks,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"