Re: [regression] TCP_MD5SIG on established sockets

2020-07-01 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 3:07 PM Eric Dumazet wrote: > Oh well, tcp_syn_options() is supposed to have the same logic. > > Maybe we have an issue with SYNCOOKIES (with MD5 + TS + SACK) > > Nice can of worms. Yes, MD5 does not like SYNCOOKIES in some cases. In this trace, S is a linux host, the

Re: [regression] TCP_MD5SIG on established sockets

2020-07-01 Thread Eric Dumazet
On Wed, Jul 1, 2020 at 5:19 AM Mathieu Desnoyers wrote: > The approach below looks good to me, but you'll also need to annotate > both tcp_md5_hash_key and tcp_md5_do_add with __no_kcsan or use > data_race(expr) to let the concurrency sanitizer know that there is > a known data race which is

Re: [regression] TCP_MD5SIG on established sockets

2020-07-01 Thread Mathieu Desnoyers
- On Jun 30, 2020, at 11:36 PM, Eric Dumazet eduma...@google.com wrote: > On Tue, Jun 30, 2020 at 7:59 PM Herbert Xu > wrote: >> >> On Tue, Jun 30, 2020 at 07:30:43PM -0700, Eric Dumazet wrote: >> > >> > I made this clear in the changelog, do we want comments all over the >> > places ? >>

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Herbert Xu
On Tue, Jun 30, 2020 at 08:36:51PM -0700, Eric Dumazet wrote: > > If I knew so many people were excited about TCP / MD5, I would have > posted all my patches on lkml ;) > > Without the smp_wmb() we would still need something to prevent KMSAN > from detecting that we read uninitialized bytes, > if

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 7:59 PM Herbert Xu wrote: > > On Tue, Jun 30, 2020 at 07:30:43PM -0700, Eric Dumazet wrote: > > > > I made this clear in the changelog, do we want comments all over the places > > ? > > Do not get me wrong, we had this bug for years and suddenly this is a > > big deal...

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Herbert Xu
On Tue, Jun 30, 2020 at 07:30:43PM -0700, Eric Dumazet wrote: > > I made this clear in the changelog, do we want comments all over the places ? > Do not get me wrong, we had this bug for years and suddenly this is a > big deal... I thought you were adding a new pair of smp_rmb/smp_wmb. If they

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Joe Perches
On Tue, 2020-06-30 at 19:30 -0700, Eric Dumazet wrote: > On Tue, Jun 30, 2020 at 7:23 PM Herbert Xu > wrote: > > On Tue, Jun 30, 2020 at 07:17:46PM -0700, Eric Dumazet wrote: > > > The main issue of the prior code was the double read of key->keylen in > > > tcp_md5_hash_key(), not that few bytes

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 7:23 PM Herbert Xu wrote: > > On Tue, Jun 30, 2020 at 07:17:46PM -0700, Eric Dumazet wrote: > > > > The main issue of the prior code was the double read of key->keylen in > > tcp_md5_hash_key(), not that few bytes could change under us. > > > > I used smp_rmb() to ease

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Herbert Xu
On Tue, Jun 30, 2020 at 07:17:46PM -0700, Eric Dumazet wrote: > > The main issue of the prior code was the double read of key->keylen in > tcp_md5_hash_key(), not that few bytes could change under us. > > I used smp_rmb() to ease backports, since old kernels had no > READ_ONCE()/WRITE_ONCE(), but

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 7:02 PM Herbert Xu wrote: > > Eric Dumazet wrote: > > > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > > index > > 810cc164f795f8e1e8ca747ed5df51bb20fec8a2..ecc0e3fabce8b03bef823cbfc5c1b0a9e24df124 > > 100644 > > --- a/net/ipv4/tcp.c > > +++ b/net/ipv4/tcp.c > > @@

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Herbert Xu
Eric Dumazet wrote: > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index > 810cc164f795f8e1e8ca747ed5df51bb20fec8a2..ecc0e3fabce8b03bef823cbfc5c1b0a9e24df124 > 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -4034,9 +4034,12 @@ EXPORT_SYMBOL(tcp_md5_hash_skb_data); > int

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 4:44 PM Mathieu Desnoyers wrote: > > - On Jun 30, 2020, at 6:38 PM, Eric Dumazet eduma...@google.com wrote: > [...] > > > > For updates of keys, it seems existing code lacks some RCU care. > > > > MD5 keys use RCU for lookups/hashes, but the replacement of a key does >

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Mathieu Desnoyers
- On Jun 30, 2020, at 6:38 PM, Eric Dumazet eduma...@google.com wrote: [...] > > For updates of keys, it seems existing code lacks some RCU care. > > MD5 keys use RCU for lookups/hashes, but the replacement of a key does > not allocate a new piece of memory. How is that RCU-safe ? Based on

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 3:07 PM Eric Dumazet wrote: > > On Tue, Jun 30, 2020 at 2:54 PM Eric Dumazet wrote: > > > > On Tue, Jun 30, 2020 at 2:23 PM Eric Dumazet wrote: > > > > > > On Tue, Jun 30, 2020 at 2:17 PM Mathieu Desnoyers > > > wrote: > > > > > > > > - On Jun 30, 2020, at 4:56 PM,

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 2:54 PM Eric Dumazet wrote: > > On Tue, Jun 30, 2020 at 2:23 PM Eric Dumazet wrote: > > > > On Tue, Jun 30, 2020 at 2:17 PM Mathieu Desnoyers > > wrote: > > > > > > - On Jun 30, 2020, at 4:56 PM, Eric Dumazet eduma...@google.com wrote: > > > > > > > On Tue, Jun 30,

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 2:23 PM Eric Dumazet wrote: > > On Tue, Jun 30, 2020 at 2:17 PM Mathieu Desnoyers > wrote: > > > > - On Jun 30, 2020, at 4:56 PM, Eric Dumazet eduma...@google.com wrote: > > > > > On Tue, Jun 30, 2020 at 1:44 PM David Miller wrote: > > >> > > >> From: Eric Dumazet >

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 2:17 PM Mathieu Desnoyers wrote: > > - On Jun 30, 2020, at 4:56 PM, Eric Dumazet eduma...@google.com wrote: > > > On Tue, Jun 30, 2020 at 1:44 PM David Miller wrote: > >> > >> From: Eric Dumazet > >> Date: Tue, 30 Jun 2020 13:39:27 -0700 > >> > >> > The (C) & (B)

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Mathieu Desnoyers
- On Jun 30, 2020, at 4:56 PM, Eric Dumazet eduma...@google.com wrote: > On Tue, Jun 30, 2020 at 1:44 PM David Miller wrote: >> >> From: Eric Dumazet >> Date: Tue, 30 Jun 2020 13:39:27 -0700 >> >> > The (C) & (B) case are certainly doable. >> > >> > A) case is more complex, I have no idea

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 1:44 PM David Miller wrote: > > From: Eric Dumazet > Date: Tue, 30 Jun 2020 13:39:27 -0700 > > > The (C) & (B) case are certainly doable. > > > > A) case is more complex, I have no idea of breakages of various TCP > > stacks if a flow got SACK > > at some point (in 3WHS)

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread David Miller
From: Eric Dumazet Date: Tue, 30 Jun 2020 13:39:27 -0700 > The (C) & (B) case are certainly doable. > > A) case is more complex, I have no idea of breakages of various TCP > stacks if a flow got SACK > at some point (in 3WHS) but suddenly becomes Reno. I agree that C and B are the easiest to

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 1:34 PM Mathieu Desnoyers wrote: > > - On Jun 30, 2020, at 3:52 PM, Linus Torvalds > torva...@linux-foundation.org wrote: > > > On Tue, Jun 30, 2020 at 12:43 PM Linus Torvalds > > wrote: > >> > [...] > > So I think it's still wrong (clearly others do change passwords

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Mathieu Desnoyers
- On Jun 30, 2020, at 4:30 PM, Eric Dumazet eduma...@google.com wrote: > On Tue, Jun 30, 2020 at 1:21 PM David Miller wrote: >> >> From: Linus Torvalds >> Date: Tue, 30 Jun 2020 12:43:21 -0700 >> >> > If you're not willing to do the work to fix it, I will revert that >> > commit. >> >>

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Mathieu Desnoyers
- On Jun 30, 2020, at 3:52 PM, Linus Torvalds torva...@linux-foundation.org wrote: > On Tue, Jun 30, 2020 at 12:43 PM Linus Torvalds > wrote: >> [...] > So I think it's still wrong (clearly others do change passwords > outside of listening state), but considering that it apparently took >

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Eric Dumazet
On Tue, Jun 30, 2020 at 1:21 PM David Miller wrote: > > From: Linus Torvalds > Date: Tue, 30 Jun 2020 12:43:21 -0700 > > > If you're not willing to do the work to fix it, I will revert that > > commit. > > Please let me handle this situation instead of making threats, this > just got reported. >

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread David Miller
From: Linus Torvalds Date: Tue, 30 Jun 2020 12:43:21 -0700 > If you're not willing to do the work to fix it, I will revert that > commit. Please let me handle this situation instead of making threats, this just got reported. Thank you.

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Linus Torvalds
On Tue, Jun 30, 2020 at 12:43 PM Linus Torvalds wrote: > > If you're not willing to do the work to fix it, I will revert that > commit. Hmm. I only now noticed that that commit is over two years old. So I think it's still wrong (clearly others do change passwords outside of listening state),

Re: [regression] TCP_MD5SIG on established sockets

2020-06-30 Thread Linus Torvalds
On Mon, Jun 29, 2020 at 1:47 PM Eric Dumazet wrote: > > If you want to be able to _change_ md5 key, this is fine by me, please > send a patch. Eric, if this change breaks existing users, then it gets reverted. That's just fundamental. No RFC's are in the lreast relevant when compared to "this

Re: [regression] TCP_MD5SIG on established sockets

2020-06-29 Thread Eric Dumazet
On Mon, Jun 29, 2020 at 12:43 PM Mathieu Desnoyers wrote: > > - On May 13, 2020, at 3:56 PM, Eric Dumazet eduma...@google.com wrote: > > > On Wed, May 13, 2020 at 12:49 PM Eric Dumazet wrote: > >> > >> > >> On Wed, May 13, 2020 at 12:38 PM Mathieu Desnoyers > >> wrote: > >> > > >> > Hi, >

Re: [regression] TCP_MD5SIG on established sockets

2020-06-29 Thread Mathieu Desnoyers
- On May 13, 2020, at 3:56 PM, Eric Dumazet eduma...@google.com wrote: > On Wed, May 13, 2020 at 12:49 PM Eric Dumazet wrote: >> >> >> On Wed, May 13, 2020 at 12:38 PM Mathieu Desnoyers >> wrote: >> > >> > Hi, >> > >> > I am reporting a regression with respect to use of >> >