* [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err()
@ 2019-02-15 21:36 Eric Dumazet
2019-02-15 21:36 ` [PATCH net 1/2] tcp: clear icsk_backoff in tcp_write_queue_purge() Eric Dumazet
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Eric Dumazet @ 2019-02-15 21:36 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Neal Cardwell, Yuchung Cheng,
soukjin bae
soukjin bae reported a crash in tcp_v4_err() that we
root caused to a missing initialization.
Second patch adds a sanity check in tcp_v4_err() to avoid
future potential problems. Ignoring an ICMP message
is probably better than crashing a machine.
Eric Dumazet (2):
tcp: clear icsk_backoff in tcp_write_queue_purge()
tcp: tcp_v4_err() should be more careful
net/ipv4/tcp.c | 2 +-
net/ipv4/tcp_ipv4.c | 5 ++++-
2 files changed, 5 insertions(+), 2 deletions(-)
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net 1/2] tcp: clear icsk_backoff in tcp_write_queue_purge()
2019-02-15 21:36 [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err() Eric Dumazet
@ 2019-02-15 21:36 ` Eric Dumazet
2019-02-16 2:21 ` Neal Cardwell
2019-02-15 21:36 ` [PATCH net 2/2] tcp: tcp_v4_err() should be more careful Eric Dumazet
2019-02-17 23:47 ` [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err() David Miller
2 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2019-02-15 21:36 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Neal Cardwell, Yuchung Cheng,
soukjin bae
soukjin bae reported a crash in tcp_v4_err() handling
ICMP_DEST_UNREACH after tcp_write_queue_head(sk)
returned a NULL pointer.
Current logic should have prevented this :
if (seq != tp->snd_una || !icsk->icsk_retransmits ||
!icsk->icsk_backoff || fastopen)
break;
Problem is the write queue might have been purged
and icsk_backoff has not been cleared.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: soukjin bae <soukjin.bae@samsung.com>
---
net/ipv4/tcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2079145a3b7c5f498af429c9a8289342e4421fca..cf3c5095c10e8e7e56621beae2f93c93de184489 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2528,6 +2528,7 @@ void tcp_write_queue_purge(struct sock *sk)
sk_mem_reclaim(sk);
tcp_clear_all_retrans_hints(tcp_sk(sk));
tcp_sk(sk)->packets_out = 0;
+ inet_csk(sk)->icsk_backoff = 0;
}
int tcp_disconnect(struct sock *sk, int flags)
@@ -2576,7 +2577,6 @@ int tcp_disconnect(struct sock *sk, int flags)
tp->write_seq += tp->max_window + 2;
if (tp->write_seq == 0)
tp->write_seq = 1;
- icsk->icsk_backoff = 0;
tp->snd_cwnd = 2;
icsk->icsk_probes_out = 0;
tp->snd_ssthresh = TCP_INFINITE_SSTHRESH;
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net 2/2] tcp: tcp_v4_err() should be more careful
2019-02-15 21:36 [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err() Eric Dumazet
2019-02-15 21:36 ` [PATCH net 1/2] tcp: clear icsk_backoff in tcp_write_queue_purge() Eric Dumazet
@ 2019-02-15 21:36 ` Eric Dumazet
2019-02-16 2:21 ` Neal Cardwell
2019-02-17 23:47 ` [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err() David Miller
2 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2019-02-15 21:36 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Neal Cardwell, Yuchung Cheng,
soukjin bae
ICMP handlers are not very often stressed, we should
make them more resilient to bugs that might surface in
the future.
If there is no packet in retransmit queue, we should
avoid a NULL deref.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: soukjin bae <soukjin.bae@samsung.com>
---
net/ipv4/tcp_ipv4.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index efc6fef692ffdca4dcdd3f4b87a837656dd66c8c..ec3cea9d68288244d8e03b655d06f91640c36ee7 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -536,12 +536,15 @@ int tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
if (sock_owned_by_user(sk))
break;
+ skb = tcp_rtx_queue_head(sk);
+ if (WARN_ON_ONCE(!skb))
+ break;
+
icsk->icsk_backoff--;
icsk->icsk_rto = tp->srtt_us ? __tcp_set_rto(tp) :
TCP_TIMEOUT_INIT;
icsk->icsk_rto = inet_csk_rto_backoff(icsk, TCP_RTO_MAX);
- skb = tcp_rtx_queue_head(sk);
tcp_mstamp_refresh(tp);
delta_us = (u32)(tp->tcp_mstamp - tcp_skb_timestamp_us(skb));
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net 2/2] tcp: tcp_v4_err() should be more careful
2019-02-15 21:36 ` [PATCH net 2/2] tcp: tcp_v4_err() should be more careful Eric Dumazet
@ 2019-02-16 2:21 ` Neal Cardwell
2019-02-16 2:31 ` Soheil Hassas Yeganeh
0 siblings, 1 reply; 9+ messages in thread
From: Neal Cardwell @ 2019-02-16 2:21 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Yuchung Cheng, soukjin bae
On Fri, Feb 15, 2019 at 4:36 PM Eric Dumazet <edumazet@google.com> wrote:
>
> ICMP handlers are not very often stressed, we should
> make them more resilient to bugs that might surface in
> the future.
>
> If there is no packet in retransmit queue, we should
> avoid a NULL deref.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: soukjin bae <soukjin.bae@samsung.com>
> ---
> net/ipv4/tcp_ipv4.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks!
neal
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/2] tcp: clear icsk_backoff in tcp_write_queue_purge()
2019-02-15 21:36 ` [PATCH net 1/2] tcp: clear icsk_backoff in tcp_write_queue_purge() Eric Dumazet
@ 2019-02-16 2:21 ` Neal Cardwell
0 siblings, 0 replies; 9+ messages in thread
From: Neal Cardwell @ 2019-02-16 2:21 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Eric Dumazet, Yuchung Cheng, soukjin bae
On Fri, Feb 15, 2019 at 4:36 PM Eric Dumazet <edumazet@google.com> wrote:
>
> soukjin bae reported a crash in tcp_v4_err() handling
> ICMP_DEST_UNREACH after tcp_write_queue_head(sk)
> returned a NULL pointer.
>
> Current logic should have prevented this :
>
> if (seq != tp->snd_una || !icsk->icsk_retransmits ||
> !icsk->icsk_backoff || fastopen)
> break;
>
> Problem is the write queue might have been purged
> and icsk_backoff has not been cleared.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: soukjin bae <soukjin.bae@samsung.com>
> ---
> net/ipv4/tcp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks!
neal
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 2/2] tcp: tcp_v4_err() should be more careful
2019-02-16 2:21 ` Neal Cardwell
@ 2019-02-16 2:31 ` Soheil Hassas Yeganeh
0 siblings, 0 replies; 9+ messages in thread
From: Soheil Hassas Yeganeh @ 2019-02-16 2:31 UTC (permalink / raw)
To: Neal Cardwell
Cc: Eric Dumazet, David S . Miller, netdev, Eric Dumazet,
Yuchung Cheng, soukjin bae
On Fri, Feb 15, 2019 at 9:21 PM Neal Cardwell <ncardwell@google.com> wrote:
>
> On Fri, Feb 15, 2019 at 4:36 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > ICMP handlers are not very often stressed, we should
> > make them more resilient to bugs that might surface in
> > the future.
> >
> > If there is no packet in retransmit queue, we should
> > avoid a NULL deref.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: soukjin bae <soukjin.bae@samsung.com>
> > ---
> > net/ipv4/tcp_ipv4.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Thanks you for the fix, Eric!
>
> Thanks!
>
> neal
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err()
2019-02-15 21:36 [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err() Eric Dumazet
2019-02-15 21:36 ` [PATCH net 1/2] tcp: clear icsk_backoff in tcp_write_queue_purge() Eric Dumazet
2019-02-15 21:36 ` [PATCH net 2/2] tcp: tcp_v4_err() should be more careful Eric Dumazet
@ 2019-02-17 23:47 ` David Miller
2019-02-18 2:20 ` Eric Dumazet
2 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2019-02-17 23:47 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, ncardwell, ycheng, soukjin.bae
From: Eric Dumazet <edumazet@google.com>
Date: Fri, 15 Feb 2019 13:36:19 -0800
> soukjin bae reported a crash in tcp_v4_err() that we
> root caused to a missing initialization.
>
> Second patch adds a sanity check in tcp_v4_err() to avoid
> future potential problems. Ignoring an ICMP message
> is probably better than crashing a machine.
Series applied, thanks Eric.
Want me to queue these up for -stable?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err()
2019-02-17 23:47 ` [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err() David Miller
@ 2019-02-18 2:20 ` Eric Dumazet
2019-02-18 19:59 ` David Miller
0 siblings, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2019-02-18 2:20 UTC (permalink / raw)
To: David Miller, edumazet
Cc: netdev, eric.dumazet, ncardwell, ycheng, soukjin.bae
On 02/17/2019 03:47 PM, David Miller wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Fri, 15 Feb 2019 13:36:19 -0800
>
>> soukjin bae reported a crash in tcp_v4_err() that we
>> root caused to a missing initialization.
>>
>> Second patch adds a sanity check in tcp_v4_err() to avoid
>> future potential problems. Ignoring an ICMP message
>> is probably better than crashing a machine.
>
> Series applied, thanks Eric.
>
> Want me to queue these up for -stable?
>
Yes please, I put no Fixes: tag because it seemed to be a day-0 bug.
Thanks !
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err()
2019-02-18 2:20 ` Eric Dumazet
@ 2019-02-18 19:59 ` David Miller
0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2019-02-18 19:59 UTC (permalink / raw)
To: eric.dumazet; +Cc: edumazet, netdev, ncardwell, ycheng, soukjin.bae
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 17 Feb 2019 18:20:21 -0800
>
>
> On 02/17/2019 03:47 PM, David Miller wrote:
>> From: Eric Dumazet <edumazet@google.com>
>> Date: Fri, 15 Feb 2019 13:36:19 -0800
>>
>>> soukjin bae reported a crash in tcp_v4_err() that we
>>> root caused to a missing initialization.
>>>
>>> Second patch adds a sanity check in tcp_v4_err() to avoid
>>> future potential problems. Ignoring an ICMP message
>>> is probably better than crashing a machine.
>>
>> Series applied, thanks Eric.
>>
>> Want me to queue these up for -stable?
>>
>
> Yes please, I put no Fixes: tag because it seemed to be a day-0 bug.
Done.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-02-18 19:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-15 21:36 [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err() Eric Dumazet
2019-02-15 21:36 ` [PATCH net 1/2] tcp: clear icsk_backoff in tcp_write_queue_purge() Eric Dumazet
2019-02-16 2:21 ` Neal Cardwell
2019-02-15 21:36 ` [PATCH net 2/2] tcp: tcp_v4_err() should be more careful Eric Dumazet
2019-02-16 2:21 ` Neal Cardwell
2019-02-16 2:31 ` Soheil Hassas Yeganeh
2019-02-17 23:47 ` [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err() David Miller
2019-02-18 2:20 ` Eric Dumazet
2019-02-18 19:59 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).