[PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Eric Dumazet]

MD5 keys are read with RCU protection, and tcp_md5_do_add()
might update in-place a prior key.

Normally, typical RCU updates would allocate a new piece
of memory. In this case only key->key and key->keylen might
be updated, and we do not care if an incoming packet could
see the old key, the new one, or some intermediate value,
since changing the key on a live flow is known to be problematic
anyway.

We only want to make sure that in the case key->keylen
is changed, cpus in tcp_md5_hash_key() wont try to use
uninitialized data, or crash because key->keylen was
read twice to feed sg_init_one() and ahash_request_set_crypt()

Fixes: 9ea88a153001 ("tcp: md5: check md5 signature without socket lock")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 net/ipv4/tcp.c      | 7 +++++--
 net/ipv4/tcp_ipv4.c | 3 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 810cc164f795f8e1e8ca747ed5df51bb20fec8a2..f111660453241692a17c881dd6dc2910a1236263 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4033,10 +4033,13 @@ EXPORT_SYMBOL(tcp_md5_hash_skb_data);
 
 int tcp_md5_hash_key(struct tcp_md5sig_pool *hp, const struct tcp_md5sig_key *key)
 {
+	u8 keylen = key->keylen;
 	struct scatterlist sg;
 
-	sg_init_one(&sg, key->key, key->keylen);
-	ahash_request_set_crypt(hp->md5_req, &sg, NULL, key->keylen);
+	smp_rmb(); /* paired with smp_wmb() in tcp_md5_do_add() */
+
+	sg_init_one(&sg, key->key, keylen);
+	ahash_request_set_crypt(hp->md5_req, &sg, NULL, keylen);
 	return crypto_ahash_update(hp->md5_req);
 }
 EXPORT_SYMBOL(tcp_md5_hash_key);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index ad6435ba6d72ffd8caf783bb25cad7ec151d6909..99916fcc15ca0be12c2c133ff40516f79e6fdf7f 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1113,6 +1113,9 @@ int tcp_md5_do_add(struct sock *sk, const union tcp_md5_addr *addr,
 	if (key) {
 		/* Pre-existing entry - just update that one. */
 		memcpy(key->key, newkey, newkeylen);
+
+		smp_wmb(); /* pairs with smp_rmb() in tcp_md5_hash_key() */
+
 		key->keylen = newkeylen;
 		return 0;
 	}
-- 
2.27.0.212.ge8ba1cc988-goog

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Mathieu Desnoyers]

----- On Jun 30, 2020, at 7:41 PM, Eric Dumazet edumazet@google.com wrote:

> MD5 keys are read with RCU protection, and tcp_md5_do_add()
> might update in-place a prior key.
> 
> Normally, typical RCU updates would allocate a new piece
> of memory. In this case only key->key and key->keylen might
> be updated, and we do not care if an incoming packet could
> see the old key, the new one, or some intermediate value,
> since changing the key on a live flow is known to be problematic
> anyway.

What makes it acceptable to observe an intermediate bogus key during the
transition ?

Thanks,

Mathieu

> 
> We only want to make sure that in the case key->keylen
> is changed, cpus in tcp_md5_hash_key() wont try to use
> uninitialized data, or crash because key->keylen was
> read twice to feed sg_init_one() and ahash_request_set_crypt()
> 
> Fixes: 9ea88a153001 ("tcp: md5: check md5 signature without socket lock")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
> net/ipv4/tcp.c      | 7 +++++--
> net/ipv4/tcp_ipv4.c | 3 +++
> 2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index
> 810cc164f795f8e1e8ca747ed5df51bb20fec8a2..f111660453241692a17c881dd6dc2910a1236263
> 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -4033,10 +4033,13 @@ EXPORT_SYMBOL(tcp_md5_hash_skb_data);
> 
> int tcp_md5_hash_key(struct tcp_md5sig_pool *hp, const struct tcp_md5sig_key
> *key)
> {
> +	u8 keylen = key->keylen;
> 	struct scatterlist sg;
> 
> -	sg_init_one(&sg, key->key, key->keylen);
> -	ahash_request_set_crypt(hp->md5_req, &sg, NULL, key->keylen);
> +	smp_rmb(); /* paired with smp_wmb() in tcp_md5_do_add() */
> +
> +	sg_init_one(&sg, key->key, keylen);
> +	ahash_request_set_crypt(hp->md5_req, &sg, NULL, keylen);
> 	return crypto_ahash_update(hp->md5_req);
> }
> EXPORT_SYMBOL(tcp_md5_hash_key);
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index
> ad6435ba6d72ffd8caf783bb25cad7ec151d6909..99916fcc15ca0be12c2c133ff40516f79e6fdf7f
> 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1113,6 +1113,9 @@ int tcp_md5_do_add(struct sock *sk, const union
> tcp_md5_addr *addr,
> 	if (key) {
> 		/* Pre-existing entry - just update that one. */
> 		memcpy(key->key, newkey, newkeylen);
> +
> +		smp_wmb(); /* pairs with smp_rmb() in tcp_md5_hash_key() */
> +
> 		key->keylen = newkeylen;
> 		return 0;
> 	}
> --
> 2.27.0.212.ge8ba1cc988-goog

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Eric Dumazet]

On Tue, Jun 30, 2020 at 4:47 PM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> ----- On Jun 30, 2020, at 7:41 PM, Eric Dumazet edumazet@google.com wrote:
>
> > MD5 keys are read with RCU protection, and tcp_md5_do_add()
> > might update in-place a prior key.
> >
> > Normally, typical RCU updates would allocate a new piece
> > of memory. In this case only key->key and key->keylen might
> > be updated, and we do not care if an incoming packet could
> > see the old key, the new one, or some intermediate value,
> > since changing the key on a live flow is known to be problematic
> > anyway.
>
> What makes it acceptable to observe an intermediate bogus key during the
> transition ?

If you change a key while packets are in flight, the result is that :

1) Either your packet has the correct key and is handled

2) Or the key do not match, packet is dropped.

 Sender will retransmit eventually.

If this was not the case, then we could not revert the patch you are
complaining about :)

>
> Thanks,
>
> Mathieu
>
> >
> > We only want to make sure that in the case key->keylen
> > is changed, cpus in tcp_md5_hash_key() wont try to use
> > uninitialized data, or crash because key->keylen was
> > read twice to feed sg_init_one() and ahash_request_set_crypt()
> >
> > Fixes: 9ea88a153001 ("tcp: md5: check md5 signature without socket lock")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > ---
> > net/ipv4/tcp.c      | 7 +++++--
> > net/ipv4/tcp_ipv4.c | 3 +++
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index
> > 810cc164f795f8e1e8ca747ed5df51bb20fec8a2..f111660453241692a17c881dd6dc2910a1236263
> > 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -4033,10 +4033,13 @@ EXPORT_SYMBOL(tcp_md5_hash_skb_data);
> >
> > int tcp_md5_hash_key(struct tcp_md5sig_pool *hp, const struct tcp_md5sig_key
> > *key)
> > {
> > +     u8 keylen = key->keylen;
> >       struct scatterlist sg;
> >
> > -     sg_init_one(&sg, key->key, key->keylen);
> > -     ahash_request_set_crypt(hp->md5_req, &sg, NULL, key->keylen);
> > +     smp_rmb(); /* paired with smp_wmb() in tcp_md5_do_add() */
> > +
> > +     sg_init_one(&sg, key->key, keylen);
> > +     ahash_request_set_crypt(hp->md5_req, &sg, NULL, keylen);
> >       return crypto_ahash_update(hp->md5_req);
> > }
> > EXPORT_SYMBOL(tcp_md5_hash_key);
> > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> > index
> > ad6435ba6d72ffd8caf783bb25cad7ec151d6909..99916fcc15ca0be12c2c133ff40516f79e6fdf7f
> > 100644
> > --- a/net/ipv4/tcp_ipv4.c
> > +++ b/net/ipv4/tcp_ipv4.c
> > @@ -1113,6 +1113,9 @@ int tcp_md5_do_add(struct sock *sk, const union
> > tcp_md5_addr *addr,
> >       if (key) {
> >               /* Pre-existing entry - just update that one. */
> >               memcpy(key->key, newkey, newkeylen);
> > +
> > +             smp_wmb(); /* pairs with smp_rmb() in tcp_md5_hash_key() */
> > +
> >               key->keylen = newkeylen;
> >               return 0;
> >       }
> > --
> > 2.27.0.212.ge8ba1cc988-goog
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Mathieu Desnoyers]

----- On Jun 30, 2020, at 7:50 PM, Eric Dumazet edumazet@google.com wrote:

> On Tue, Jun 30, 2020 at 4:47 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> ----- On Jun 30, 2020, at 7:41 PM, Eric Dumazet edumazet@google.com wrote:
>>
>> > MD5 keys are read with RCU protection, and tcp_md5_do_add()
>> > might update in-place a prior key.
>> >
>> > Normally, typical RCU updates would allocate a new piece
>> > of memory. In this case only key->key and key->keylen might
>> > be updated, and we do not care if an incoming packet could
>> > see the old key, the new one, or some intermediate value,
>> > since changing the key on a live flow is known to be problematic
>> > anyway.
>>
>> What makes it acceptable to observe an intermediate bogus key during the
>> transition ?
> 
> If you change a key while packets are in flight, the result is that :
> 
> 1) Either your packet has the correct key and is handled
> 
> 2) Or the key do not match, packet is dropped.
> 
> Sender will retransmit eventually.

This train of thoughts seem to apply to incoming traffic, what about outgoing ?

> 
> If this was not the case, then we could not revert the patch you are
> complaining about :)

Please let me know where I'm incorrect with the following scenario:

- Local peer is a Linux host, which supports only a single MD5 key
  per socket at any given time.
- Remote peer is a Ericsson/Redback device allowing 2 passwords (old/new)
  to co-exist until both sides are OK with the new password.

The local peer updates the MD5 password for a socket in parallel with
packets flow. If we guarantee that no intermediate bogus key state is
observable, the flow going out of the Linux peer should always be seen as
valid, with either the old or the new key.

Allowing an intermediate bogus key to be observable means this introduces
a race condition during which the remote peer can observe a corrupted key.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Eric Dumazet]

On Tue, Jun 30, 2020 at 5:27 PM Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> ----- On Jun 30, 2020, at 7:50 PM, Eric Dumazet edumazet@google.com wrote:
>
> > On Tue, Jun 30, 2020 at 4:47 PM Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> >>
> >> ----- On Jun 30, 2020, at 7:41 PM, Eric Dumazet edumazet@google.com wrote:
> >>
> >> > MD5 keys are read with RCU protection, and tcp_md5_do_add()
> >> > might update in-place a prior key.
> >> >
> >> > Normally, typical RCU updates would allocate a new piece
> >> > of memory. In this case only key->key and key->keylen might
> >> > be updated, and we do not care if an incoming packet could
> >> > see the old key, the new one, or some intermediate value,
> >> > since changing the key on a live flow is known to be problematic
> >> > anyway.
> >>
> >> What makes it acceptable to observe an intermediate bogus key during the
> >> transition ?
> >
> > If you change a key while packets are in flight, the result is that :
> >
> > 1) Either your packet has the correct key and is handled
> >
> > 2) Or the key do not match, packet is dropped.
> >
> > Sender will retransmit eventually.
>
> This train of thoughts seem to apply to incoming traffic, what about outgoing ?


Outgoing path is protected by the socket lock.

You can not change the TCP MD5 key while xmit is in progress.

>
> >
> > If this was not the case, then we could not revert the patch you are
> > complaining about :)
>
> Please let me know where I'm incorrect with the following scenario:
>
> - Local peer is a Linux host, which supports only a single MD5 key
>   per socket at any given time.
> - Remote peer is a Ericsson/Redback device allowing 2 passwords (old/new)
>   to co-exist until both sides are OK with the new password.
>
> The local peer updates the MD5 password for a socket in parallel with
> packets flow. If we guarantee that no intermediate bogus key state is
> observable, the flow going out of the Linux peer should always be seen as
> valid, with either the old or the new key.

Linux has one key.

If your peer sends a packet with the old key, you will drop the
packet, no matter how hard you try.


>
> Allowing an intermediate bogus key to be observable means this introduces
> a race condition during which the remote peer can observe a corrupted key.

Race condition is there, even if you change the whole TCP MD5 key atomically.

If another cpu receives a packet while you are changing the key, you
definitely can not predict
if the packet is going to be dropped or not.


>
> Thanks,
>
> Mathieu
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Mathieu Desnoyers]

----- On Jun 30, 2020, at 8:34 PM, Eric Dumazet edumazet@google.com wrote:

> On Tue, Jun 30, 2020 at 5:27 PM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> ----- On Jun 30, 2020, at 7:50 PM, Eric Dumazet edumazet@google.com wrote:
>>
>> > On Tue, Jun 30, 2020 at 4:47 PM Mathieu Desnoyers
>> > <mathieu.desnoyers@efficios.com> wrote:
>> >>
>> >> ----- On Jun 30, 2020, at 7:41 PM, Eric Dumazet edumazet@google.com wrote:
>> >>
>> >> > MD5 keys are read with RCU protection, and tcp_md5_do_add()
>> >> > might update in-place a prior key.
>> >> >
>> >> > Normally, typical RCU updates would allocate a new piece
>> >> > of memory. In this case only key->key and key->keylen might
>> >> > be updated, and we do not care if an incoming packet could
>> >> > see the old key, the new one, or some intermediate value,
>> >> > since changing the key on a live flow is known to be problematic
>> >> > anyway.
>> >>
>> >> What makes it acceptable to observe an intermediate bogus key during the
>> >> transition ?
>> >
>> > If you change a key while packets are in flight, the result is that :
>> >
>> > 1) Either your packet has the correct key and is handled
>> >
>> > 2) Or the key do not match, packet is dropped.
>> >
>> > Sender will retransmit eventually.
>>
>> This train of thoughts seem to apply to incoming traffic, what about outgoing ?
> 
> 
> Outgoing path is protected by the socket lock.
> 
> You can not change the TCP MD5 key while xmit is in progress.

Allright, this is the part I missed, thanks!

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Hideaki Yoshifuji]

Hi,

2020年7月1日(水) 8:41 Eric Dumazet <edumazet@google.com>:
:
> We only want to make sure that in the case key->keylen
> is changed, cpus in tcp_md5_hash_key() wont try to use
> uninitialized data, or crash because key->keylen was
> read twice to feed sg_init_one() and ahash_request_set_crypt()
>
> Fixes: 9ea88a153001 ("tcp: md5: check md5 signature without socket lock")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
>  net/ipv4/tcp.c      | 7 +++++--
>  net/ipv4/tcp_ipv4.c | 3 +++
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 810cc164f795f8e1e8ca747ed5df51bb20fec8a2..f111660453241692a17c881dd6dc2910a1236263 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -4033,10 +4033,13 @@ EXPORT_SYMBOL(tcp_md5_hash_skb_data);
>
>  int tcp_md5_hash_key(struct tcp_md5sig_pool *hp, const struct tcp_md5sig_key *key)
>  {
> +       u8 keylen = key->keylen;
>         struct scatterlist sg;

ACCESS_ONCE here, no?

--yoshfuji

>
> -       sg_init_one(&sg, key->key, key->keylen);
> -       ahash_request_set_crypt(hp->md5_req, &sg, NULL, key->keylen);
> +       smp_rmb(); /* paired with smp_wmb() in tcp_md5_do_add() */
> +
> +       sg_init_one(&sg, key->key, keylen);
> +       ahash_request_set_crypt(hp->md5_req, &sg, NULL, keylen);
>         return crypto_ahash_update(hp->md5_req);
>  }
>  EXPORT_SYMBOL(tcp_md5_hash_key);
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index ad6435ba6d72ffd8caf783bb25cad7ec151d6909..99916fcc15ca0be12c2c133ff40516f79e6fdf7f 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1113,6 +1113,9 @@ int tcp_md5_do_add(struct sock *sk, const union tcp_md5_addr *addr,
>         if (key) {
>                 /* Pre-existing entry - just update that one. */
>                 memcpy(key->key, newkey, newkeylen);
> +
> +               smp_wmb(); /* pairs with smp_rmb() in tcp_md5_hash_key() */
> +
>                 key->keylen = newkeylen;
>                 return 0;
>         }
> --
> 2.27.0.212.ge8ba1cc988-goog
>

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Eric Dumazet]

On Tue, Jun 30, 2020 at 5:53 PM Hideaki Yoshifuji
<hideaki.yoshifuji@miraclelinux.com> wrote:
>
> Hi,
>
> 2020年7月1日(水) 8:41 Eric Dumazet <edumazet@google.com>:
> :
> > We only want to make sure that in the case key->keylen
> > is changed, cpus in tcp_md5_hash_key() wont try to use
> > uninitialized data, or crash because key->keylen was
> > read twice to feed sg_init_one() and ahash_request_set_crypt()
> >
> > Fixes: 9ea88a153001 ("tcp: md5: check md5 signature without socket lock")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > ---
> >  net/ipv4/tcp.c      | 7 +++++--
> >  net/ipv4/tcp_ipv4.c | 3 +++
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index 810cc164f795f8e1e8ca747ed5df51bb20fec8a2..f111660453241692a17c881dd6dc2910a1236263 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -4033,10 +4033,13 @@ EXPORT_SYMBOL(tcp_md5_hash_skb_data);
> >
> >  int tcp_md5_hash_key(struct tcp_md5sig_pool *hp, const struct tcp_md5sig_key *key)
> >  {
> > +       u8 keylen = key->keylen;
> >         struct scatterlist sg;
>
> ACCESS_ONCE here, no?

Not needed, the smp_rmb() barrier is stronger.

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[Mathieu Desnoyers]

----- On Jun 30, 2020, at 8:55 PM, Eric Dumazet edumazet@google.com wrote:

> On Tue, Jun 30, 2020 at 5:53 PM Hideaki Yoshifuji
> <hideaki.yoshifuji@miraclelinux.com> wrote:
>>
>> Hi,
>>
>> 2020年7月1日(水) 8:41 Eric Dumazet <edumazet@google.com>:
>> :
>> > We only want to make sure that in the case key->keylen
>> > is changed, cpus in tcp_md5_hash_key() wont try to use
>> > uninitialized data, or crash because key->keylen was
>> > read twice to feed sg_init_one() and ahash_request_set_crypt()
>> >
>> > Fixes: 9ea88a153001 ("tcp: md5: check md5 signature without socket lock")
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> > ---
>> >  net/ipv4/tcp.c      | 7 +++++--
>> >  net/ipv4/tcp_ipv4.c | 3 +++
>> >  2 files changed, 8 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> > index
>> > 810cc164f795f8e1e8ca747ed5df51bb20fec8a2..f111660453241692a17c881dd6dc2910a1236263
>> > 100644
>> > --- a/net/ipv4/tcp.c
>> > +++ b/net/ipv4/tcp.c
>> > @@ -4033,10 +4033,13 @@ EXPORT_SYMBOL(tcp_md5_hash_skb_data);
>> >
>> >  int tcp_md5_hash_key(struct tcp_md5sig_pool *hp, const struct tcp_md5sig_key
>> >  *key)
>> >  {
>> > +       u8 keylen = key->keylen;
>> >         struct scatterlist sg;
>>
>> ACCESS_ONCE here, no?
> 
> Not needed, the smp_rmb() barrier is stronger.

ACCESS_ONCE() is now deprecated in favor of READ_ONCE()/WRITE_ONCE(),
which are needed here. smp_rmb()/smp_wmb() are needed in addition, but
have a different purpose. smp_rmb() only guarantees ordering, but does
nothing to prevent the compiler from turning the load into something
unexpected. Likewise for WRITE_ONCE. See linux/compiler.h READ_ONCE and
WRITE_ONCE comment:

"2) Ensuring that the compiler does not fold, spindle, or otherwise
mutilate accesses that either do not require ordering or that interact
with an explicit memory barrier or atomic instruction that provides the
required ordering."

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH net] tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()

[David Miller]

From: Eric Dumazet <edumazet@google.com>
Date: Tue, 30 Jun 2020 16:41:01 -0700

> MD5 keys are read with RCU protection, and tcp_md5_do_add()
> might update in-place a prior key.
> 
> Normally, typical RCU updates would allocate a new piece
> of memory. In this case only key->key and key->keylen might
> be updated, and we do not care if an incoming packet could
> see the old key, the new one, or some intermediate value,
> since changing the key on a live flow is known to be problematic
> anyway.
> 
> We only want to make sure that in the case key->keylen
> is changed, cpus in tcp_md5_hash_key() wont try to use
> uninitialized data, or crash because key->keylen was
> read twice to feed sg_init_one() and ahash_request_set_crypt()
> 
> Fixes: 9ea88a153001 ("tcp: md5: check md5 signature without socket lock")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

Applied and queued up for -stable, thanks Eric.