When net.ipv4.tcp_syncookies=1 and syn flood is happened,
cookie_v4_check tries to redo what tcp_v4_send_synack did,
rsk_window_clamp will be changed if SOCK_RCVBUF is set
by user, which will make rcv_wscale is different, the client
still operates with initial window scale and can overshot
granted window, the client use the initial scale but local
server use new scale to advertise window value, and session
work abnormally.
Signed-off-by: Mao Wenan <[email protected]>
---
net/ipv4/syncookies.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 6ac473b..57ce317 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
/* Try to redo what tcp_v4_send_synack did. */
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = tcp_full_space(sk);
tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
--
1.8.3.1
When net.ipv4.tcp_syncookies=1 and syn flood is happened,
cookie_v4_check or cookie_v6_check tries to redo what
tcp_v4_send_synack or tcp_v6_send_synack did,
rsk_window_clamp will be changed if SOCK_RCVBUF is set,
which will make rcv_wscale is different, the client
still operates with initial window scale and can overshot
granted window, the client use the initial scale but local
server use new scale to advertise window value, and session
work abnormally.
Signed-off-by: Mao Wenan <[email protected]>
---
v2: fix for ipv6.
net/ipv4/syncookies.c | 4 ++++
net/ipv6/syncookies.c | 5 +++++
2 files changed, 9 insertions(+)
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 6ac473b..57ce317 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
/* Try to redo what tcp_v4_send_synack did. */
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = tcp_full_space(sk);
tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index e796a64..c041360 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -241,6 +241,11 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
}
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW);
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = tcp_full_space(sk);
+
tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
ireq->wscale_ok, &rcv_wscale,
--
1.8.3.1
On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <[email protected]> wrote:
>
> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
> cookie_v4_check or cookie_v6_check tries to redo what
> tcp_v4_send_synack or tcp_v6_send_synack did,
> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
> which will make rcv_wscale is different, the client
> still operates with initial window scale and can overshot
> granted window, the client use the initial scale but local
> server use new scale to advertise window value, and session
> work abnormally.
What is not working exactly ?
Sending a 'big wscale' should not really matter, unless perhaps there
is a buggy stack at the remote end ?
>
> Signed-off-by: Mao Wenan <[email protected]>
> ---
> v2: fix for ipv6.
> net/ipv4/syncookies.c | 4 ++++
> net/ipv6/syncookies.c | 5 +++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 6ac473b..57ce317 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>
> /* Try to redo what tcp_v4_send_synack did. */
> req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
> + /* limit the window selection if the user enforce a smaller rx buffer */
> + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
> + (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0))
> + req->rsk_window_clamp = tcp_full_space(sk);
This seems not needed to me.
We call tcp_select_initial_window() with tcp_full_space(sk) passed as
the 2nd parameter.
tcp_full_space(sk) will then apply :
space = min(*window_clamp, space);
Please cook a packetdrill test to demonstrate what you are seeing ?
在 2020/11/9 下午5:56, Eric Dumazet 写道:
> On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <[email protected]> wrote:
>>
>> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
>> cookie_v4_check or cookie_v6_check tries to redo what
>> tcp_v4_send_synack or tcp_v6_send_synack did,
>> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
>> which will make rcv_wscale is different, the client
>> still operates with initial window scale and can overshot
>> granted window, the client use the initial scale but local
>> server use new scale to advertise window value, and session
>> work abnormally.
>
> What is not working exactly ?
>
> Sending a 'big wscale' should not really matter, unless perhaps there
> is a buggy stack at the remote end ?
1)in tcp_v4_send_synack, if SO_RCVBUF is set and
tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to
tcp_select_initial_window, rcv_wscale will be zero, and send to client,
the client consider wscale is 0;
2)when ack is back from client, if there is no this patch,
req->rsk_window_clamp is 0, and pass to tcp_select_initial_window,
wscale will be 7, this new rcv_wscale is no way to advertise to client.
3)if server send rcv_wind to client with window=63, it consider the real
window is 63*2^7=8064, but client consider the server window is only
63*2^0=63, it can't send big packet to server, and the send-q of client
is full.
>
>>
>> Signed-off-by: Mao Wenan <[email protected]>
>> ---
>> v2: fix for ipv6.
>> net/ipv4/syncookies.c | 4 ++++
>> net/ipv6/syncookies.c | 5 +++++
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
>> index 6ac473b..57ce317 100644
>> --- a/net/ipv4/syncookies.c
>> +++ b/net/ipv4/syncookies.c
>> @@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>>
>> /* Try to redo what tcp_v4_send_synack did. */
>> req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
>> + /* limit the window selection if the user enforce a smaller rx buffer */
>> + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
>> + (req->rsk_window_clamp > tcp_full_space(sk) || req->rsk_window_clamp == 0))
>> + req->rsk_window_clamp = tcp_full_space(sk);
>
> This seems not needed to me.
>
> We call tcp_select_initial_window() with tcp_full_space(sk) passed as
> the 2nd parameter.
>
> tcp_full_space(sk) will then apply :
>
> space = min(*window_clamp, space);
if cookie_v4_check pass window_clamp=0 to tcp_select_initial_window, it
will set window_clamp to max value.
(*window_clamp) = (U16_MAX << TCP_MAX_WSCALE);
but space will fetch from sysctl_rmem_max and sysctl_tcp_rmem[2] which
is also big value.
space = max_t(u32, space, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]);
space = max_t(u32, space, sysctl_rmem_max);
Then,space = min(*window_clamp, space) is a big value, lead wscale to 7,
is different from tcp_v4_send_synack.
>
> Please cook a packetdrill test to demonstrate what you are seeing ?
>
I have real environment and reproduce this case, this patch can fix
that, i will try to use packetdrill with syn cookies and syn flood happen.
在 2020/11/9 下午6:12, Mao Wenan 写道:
>
>
> 在 2020/11/9 下午5:56, Eric Dumazet 写道:
>> On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan
>> <[email protected]> wrote:
>>>
>>> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
>>> cookie_v4_check or cookie_v6_check tries to redo what
>>> tcp_v4_send_synack or tcp_v6_send_synack did,
>>> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
>>> which will make rcv_wscale is different, the client
>>> still operates with initial window scale and can overshot
>>> granted window, the client use the initial scale but local
>>> server use new scale to advertise window value, and session
>>> work abnormally.
>>
>> What is not working exactly ?
>>
>> Sending a 'big wscale' should not really matter, unless perhaps there
>> is a buggy stack at the remote end ?
> 1)in tcp_v4_send_synack, if SO_RCVBUF is set and
> tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to
> tcp_select_initial_window, rcv_wscale will be zero, and send to client,
> the client consider wscale is 0;
> 2)when ack is back from client, if there is no this patch,
> req->rsk_window_clamp is 0, and pass to tcp_select_initial_window,
> wscale will be 7, this new rcv_wscale is no way to advertise to client.
> 3)if server send rcv_wind to client with window=63, it consider the real
> window is 63*2^7=8064, but client consider the server window is only
> 63*2^0=63, it can't send big packet to server, and the send-q of client
> is full.
>
>
>>
>>>
>>> Signed-off-by: Mao Wenan <[email protected]>
>>> ---
>>> v2: fix for ipv6.
>>> net/ipv4/syncookies.c | 4 ++++
>>> net/ipv6/syncookies.c | 5 +++++
>>> 2 files changed, 9 insertions(+)
>>>
>>> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
>>> index 6ac473b..57ce317 100644
>>> --- a/net/ipv4/syncookies.c
>>> +++ b/net/ipv4/syncookies.c
>>> @@ -427,6 +427,10 @@ struct sock *cookie_v4_check(struct sock *sk,
>>> struct sk_buff *skb)
>>>
>>> /* Try to redo what tcp_v4_send_synack did. */
>>> req->rsk_window_clamp = tp->window_clamp ?
>>> :dst_metric(&rt->dst, RTAX_WINDOW);
>>> + /* limit the window selection if the user enforce a smaller
>>> rx buffer */
>>> + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
>>> + (req->rsk_window_clamp > tcp_full_space(sk) ||
>>> req->rsk_window_clamp == 0))
>>> + req->rsk_window_clamp = tcp_full_space(sk);
>>
>> This seems not needed to me.
>>
>> We call tcp_select_initial_window() with tcp_full_space(sk) passed as
>> the 2nd parameter.
>>
>> tcp_full_space(sk) will then apply :
>>
>> space = min(*window_clamp, space);
>
> if cookie_v4_check pass window_clamp=0 to tcp_select_initial_window, it
> will set window_clamp to max value.
> (*window_clamp) = (U16_MAX << TCP_MAX_WSCALE);
window_clamp=0 is from
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW);
and if SO_RCVBUF is set and equal to 65535,req->rsk_window_clamp will be
65535.
req->rsk_window_clamp = tcp_full_space(sk);
>
> but space will fetch from sysctl_rmem_max and sysctl_tcp_rmem[2] which
> is also big value.
> space = max_t(u32, space, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]);
> space = max_t(u32, space, sysctl_rmem_max);
>
> Then,space = min(*window_clamp, space) is a big value, lead wscale to 7,
> is different from tcp_v4_send_synack.
>
>
>>
>> Please cook a packetdrill test to demonstrate what you are seeing ?
>>
> I have real environment and reproduce this case, this patch can fix
> that, i will try to use packetdrill with syn cookies and syn flood happen.
On Mon, Nov 9, 2020 at 11:12 AM Mao Wenan <[email protected]> wrote:
>
>
>
> 在 2020/11/9 下午5:56, Eric Dumazet 写道:
> > On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <[email protected]> wrote:
> >>
> >> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
> >> cookie_v4_check or cookie_v6_check tries to redo what
> >> tcp_v4_send_synack or tcp_v6_send_synack did,
> >> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
> >> which will make rcv_wscale is different, the client
> >> still operates with initial window scale and can overshot
> >> granted window, the client use the initial scale but local
> >> server use new scale to advertise window value, and session
> >> work abnormally.
> >
> > What is not working exactly ?
> >
> > Sending a 'big wscale' should not really matter, unless perhaps there
> > is a buggy stack at the remote end ?
> 1)in tcp_v4_send_synack, if SO_RCVBUF is set and
> tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to
> tcp_select_initial_window, rcv_wscale will be zero, and send to client,
> the client consider wscale is 0;
> 2)when ack is back from client, if there is no this patch,
> req->rsk_window_clamp is 0, and pass to tcp_select_initial_window,
> wscale will be 7, this new rcv_wscale is no way to advertise to client.
> 3)if server send rcv_wind to client with window=63, it consider the real
> window is 63*2^7=8064, but client consider the server window is only
> 63*2^0=63, it can't send big packet to server, and the send-q of client
> is full.
>
I see, please change your patches so that tcp_full_space() is used _once_
listener sk_rcvbuf can change under us.
I really have no idea how window can be set to 63, so please send us
the packetdrill test once you have it.
Packetdrill test would be :
// Force syncookies
`sysctl -q net.ipv4.tcp_syncookies=2`
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [2048], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 32792 <mss 1000,sackOK,TS val 100 ecr 0,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 4000 ecr 100,nop,wscale 0>
+.1 < . 1:1(0) ack 1 win 1024 <nop,nop,TS val 200 ecr 4000>
+0 accept(3, ..., ...) = 4
+0 %{ assert tcpi_snd_wscale == 0, tcpi_snd_wscale }%
On Mon, Nov 9, 2020 at 12:02 PM Eric Dumazet <[email protected]> wrote:
>
> On Mon, Nov 9, 2020 at 11:12 AM Mao Wenan <[email protected]> wrote:
> >
> >
> >
> > 在 2020/11/9 下午5:56, Eric Dumazet 写道:
> > > On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <[email protected]> wrote:
> > >>
> > >> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
> > >> cookie_v4_check or cookie_v6_check tries to redo what
> > >> tcp_v4_send_synack or tcp_v6_send_synack did,
> > >> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
> > >> which will make rcv_wscale is different, the client
> > >> still operates with initial window scale and can overshot
> > >> granted window, the client use the initial scale but local
> > >> server use new scale to advertise window value, and session
> > >> work abnormally.
> > >
> > > What is not working exactly ?
> > >
> > > Sending a 'big wscale' should not really matter, unless perhaps there
> > > is a buggy stack at the remote end ?
> > 1)in tcp_v4_send_synack, if SO_RCVBUF is set and
> > tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to
> > tcp_select_initial_window, rcv_wscale will be zero, and send to client,
> > the client consider wscale is 0;
> > 2)when ack is back from client, if there is no this patch,
> > req->rsk_window_clamp is 0, and pass to tcp_select_initial_window,
> > wscale will be 7, this new rcv_wscale is no way to advertise to client.
> > 3)if server send rcv_wind to client with window=63, it consider the real
> > window is 63*2^7=8064, but client consider the server window is only
> > 63*2^0=63, it can't send big packet to server, and the send-q of client
> > is full.
> >
>
> I see, please change your patches so that tcp_full_space() is used _once_
>
> listener sk_rcvbuf can change under us.
>
> I really have no idea how window can be set to 63, so please send us
> the packetdrill test once you have it.
On Mon, Nov 9, 2020 at 12:41 PM Eric Dumazet <[email protected]> wrote:
>
> Packetdrill test would be :
>
> // Force syncookies
> `sysctl -q net.ipv4.tcp_syncookies=2`
>
> 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [2048], 4) = 0
> +0 bind(3, ..., ...) = 0
> +0 listen(3, 1) = 0
>
> +0 < S 0:0(0) win 32792 <mss 1000,sackOK,TS val 100 ecr 0,nop,wscale 7>
> +0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 4000 ecr 100,nop,wscale 0>
> +.1 < . 1:1(0) ack 1 win 1024 <nop,nop,TS val 200 ecr 4000>
> +0 accept(3, ..., ...) = 4
> +0 %{ assert tcpi_snd_wscale == 0, tcpi_snd_wscale }%
>
Also, please add to your next submission an appropriate Fixes: tag :
Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's
rcv-buffer via setsockopt")
> On Mon, Nov 9, 2020 at 12:02 PM Eric Dumazet <[email protected]> wrote:
> >
> > On Mon, Nov 9, 2020 at 11:12 AM Mao Wenan <[email protected]> wrote:
> > >
> > >
> > >
> > > 在 2020/11/9 下午5:56, Eric Dumazet 写道:
> > > > On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <[email protected]> wrote:
> > > >>
> > > >> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
> > > >> cookie_v4_check or cookie_v6_check tries to redo what
> > > >> tcp_v4_send_synack or tcp_v6_send_synack did,
> > > >> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
> > > >> which will make rcv_wscale is different, the client
> > > >> still operates with initial window scale and can overshot
> > > >> granted window, the client use the initial scale but local
> > > >> server use new scale to advertise window value, and session
> > > >> work abnormally.
> > > >
> > > > What is not working exactly ?
> > > >
> > > > Sending a 'big wscale' should not really matter, unless perhaps there
> > > > is a buggy stack at the remote end ?
> > > 1)in tcp_v4_send_synack, if SO_RCVBUF is set and
> > > tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to
> > > tcp_select_initial_window, rcv_wscale will be zero, and send to client,
> > > the client consider wscale is 0;
> > > 2)when ack is back from client, if there is no this patch,
> > > req->rsk_window_clamp is 0, and pass to tcp_select_initial_window,
> > > wscale will be 7, this new rcv_wscale is no way to advertise to client.
> > > 3)if server send rcv_wind to client with window=63, it consider the real
> > > window is 63*2^7=8064, but client consider the server window is only
> > > 63*2^0=63, it can't send big packet to server, and the send-q of client
> > > is full.
> > >
> >
> > I see, please change your patches so that tcp_full_space() is used _once_
> >
> > listener sk_rcvbuf can change under us.
> >
> > I really have no idea how window can be set to 63, so please send us
> > the packetdrill test once you have it.
在 2020/11/9 下午10:01, Eric Dumazet 写道:
> On Mon, Nov 9, 2020 at 12:41 PM Eric Dumazet <[email protected]> wrote:
>>
>> Packetdrill test would be :
>>
>> // Force syncookies
>> `sysctl -q net.ipv4.tcp_syncookies=2`
>>
>> 0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
>> +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
>> +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [2048], 4) = 0
>> +0 bind(3, ..., ...) = 0
>> +0 listen(3, 1) = 0
>>
>> +0 < S 0:0(0) win 32792 <mss 1000,sackOK,TS val 100 ecr 0,nop,wscale 7>
>> +0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 4000 ecr 100,nop,wscale 0>
>> +.1 < . 1:1(0) ack 1 win 1024 <nop,nop,TS val 200 ecr 4000>
>> +0 accept(3, ..., ...) = 4
>> +0 %{ assert tcpi_snd_wscale == 0, tcpi_snd_wscale }%
>>
>
> Also, please add to your next submission an appropriate Fixes: tag :
>
> Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's
> rcv-buffer via setsockopt")
OK, thanks, I can reproduce wscale=0 with your packetdrill, and I will
send v3 with the fixes tag.
>
>> On Mon, Nov 9, 2020 at 12:02 PM Eric Dumazet <[email protected]> wrote:
>>>
>>> On Mon, Nov 9, 2020 at 11:12 AM Mao Wenan <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>> 在 2020/11/9 下午5:56, Eric Dumazet 写道:
>>>>> On Mon, Nov 9, 2020 at 10:33 AM Mao Wenan <[email protected]> wrote:
>>>>>>
>>>>>> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
>>>>>> cookie_v4_check or cookie_v6_check tries to redo what
>>>>>> tcp_v4_send_synack or tcp_v6_send_synack did,
>>>>>> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
>>>>>> which will make rcv_wscale is different, the client
>>>>>> still operates with initial window scale and can overshot
>>>>>> granted window, the client use the initial scale but local
>>>>>> server use new scale to advertise window value, and session
>>>>>> work abnormally.
>>>>>
>>>>> What is not working exactly ?
>>>>>
>>>>> Sending a 'big wscale' should not really matter, unless perhaps there
>>>>> is a buggy stack at the remote end ?
>>>> 1)in tcp_v4_send_synack, if SO_RCVBUF is set and
>>>> tcp_full_space(sk)=65535, pass req->rsk_window_clamp=65535 to
>>>> tcp_select_initial_window, rcv_wscale will be zero, and send to client,
>>>> the client consider wscale is 0;
>>>> 2)when ack is back from client, if there is no this patch,
>>>> req->rsk_window_clamp is 0, and pass to tcp_select_initial_window,
>>>> wscale will be 7, this new rcv_wscale is no way to advertise to client.
>>>> 3)if server send rcv_wind to client with window=63, it consider the real
>>>> window is 63*2^7=8064, but client consider the server window is only
>>>> 63*2^0=63, it can't send big packet to server, and the send-q of client
>>>> is full.
>>>>
>>>
>>> I see, please change your patches so that tcp_full_space() is used _once_
>>>
>>> listener sk_rcvbuf can change under us.
>>>
>>> I really have no idea how window can be set to 63, so please send us
>>> the packetdrill test once you have it.
When net.ipv4.tcp_syncookies=1 and syn flood is happened,
cookie_v4_check or cookie_v6_check tries to redo what
tcp_v4_send_synack or tcp_v6_send_synack did,
rsk_window_clamp will be changed if SOCK_RCVBUF is set,
which will make rcv_wscale is different, the client
still operates with initial window scale and can overshot
granted window, the client use the initial scale but local
server use new scale to advertise window value, and session
work abnormally.
Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's
rcv-buffer via setsockopt")
Signed-off-by: Mao Wenan <[email protected]>
---
v3: add local variable full_space, add fixes tag.
v2: fix for ipv6.
net/ipv4/syncookies.c | 7 ++++++-
net/ipv6/syncookies.c | 8 +++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 6ac473b..eea4698 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -327,6 +327,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
struct inet_request_sock *ireq;
struct tcp_request_sock *treq;
struct tcp_sock *tp = tcp_sk(sk);
+ int full_space = tcp_full_space(sk);
const struct tcphdr *th = tcp_hdr(skb);
__u32 cookie = ntohl(th->ack_seq) - 1;
struct sock *ret = sk;
@@ -427,8 +428,12 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
/* Try to redo what tcp_v4_send_synack did. */
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = full_space;
- tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
+ tcp_select_initial_window(sk, full_space, req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
ireq->wscale_ok, &rcv_wscale,
dst_metric(&rt->dst, RTAX_INITRWND));
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index e796a64..5b09bb6 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -132,6 +132,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
struct tcp_request_sock *treq;
struct ipv6_pinfo *np = inet6_sk(sk);
struct tcp_sock *tp = tcp_sk(sk);
+ int full_space = tcp_full_space(sk);
const struct tcphdr *th = tcp_hdr(skb);
__u32 cookie = ntohl(th->ack_seq) - 1;
struct sock *ret = sk;
@@ -241,7 +242,12 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
}
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW);
- tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = full_space;
+
+ tcp_select_initial_window(sk, full_space, req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
ireq->wscale_ok, &rcv_wscale,
dst_metric(dst, RTAX_INITRWND));
--
1.8.3.1
On Mon, Nov 9, 2020 at 5:54 PM Mao Wenan <[email protected]> wrote:
>
> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
> cookie_v4_check or cookie_v6_check tries to redo what
> tcp_v4_send_synack or tcp_v6_send_synack did,
> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
> which will make rcv_wscale is different, the client
> still operates with initial window scale and can overshot
> granted window, the client use the initial scale but local
> server use new scale to advertise window value, and session
> work abnormally.
>
> Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's
> rcv-buffer via setsockopt")
Please put this tag in a single line (no line wrap)
And do not add an empty line after it.
The Fixes: tag is part of the official tags, all grouped together.
>
> Signed-off-by: Mao Wenan <[email protected]>
> ---
> v3: add local variable full_space, add fixes tag.
> v2: fix for ipv6.
> net/ipv4/syncookies.c | 7 ++++++-
> net/ipv6/syncookies.c | 8 +++++++-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 6ac473b..eea4698 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -327,6 +327,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
> struct inet_request_sock *ireq;
> struct tcp_request_sock *treq;
> struct tcp_sock *tp = tcp_sk(sk);
> + int full_space = tcp_full_space(sk);
Please delay the actual call to tcp_full_space() until we need it.
If a packet does not validate the cookie, no need to init @full_space.
> const struct tcphdr *th = tcp_hdr(skb);
> __u32 cookie = ntohl(th->ack_seq) - 1;
> struct sock *ret = sk;
> @@ -427,8 +428,12 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>
> /* Try to redo what tcp_v4_send_synack did. */
> req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
> + /* limit the window selection if the user enforce a smaller rx buffer */
eg:
full_space = tcp_full_space(sk);
> + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
> + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0))
> + req->rsk_window_clamp = full_space;
>
> - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
> + tcp_select_initial_window(sk, full_space, req->mss,
> &req->rsk_rcv_wnd, &req->rsk_window_clamp,
> ireq->wscale_ok, &rcv_wscale,
> dst_metric(&rt->dst, RTAX_INITRWND));
> diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
> index e796a64..5b09bb6 100644
> --- a/net/ipv6/syncookies.c
> +++ b/net/ipv6/syncookies.c
> @@ -132,6 +132,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
> struct tcp_request_sock *treq;
> struct ipv6_pinfo *np = inet6_sk(sk);
> struct tcp_sock *tp = tcp_sk(sk);
> + int full_space = tcp_full_space(sk);
Same remark here.
> const struct tcphdr *th = tcp_hdr(skb);
> __u32 cookie = ntohl(th->ack_seq) - 1;
> struct sock *ret = sk;
> @@ -241,7 +242,12 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
> }
>
> req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW);
> - tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
> + /* limit the window selection if the user enforce a smaller rx buffer */
> + if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
> + (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0))
> + req->rsk_window_clamp = full_space;
> +
> + tcp_select_initial_window(sk, full_space, req->mss,
> &req->rsk_rcv_wnd, &req->rsk_window_clamp,
> ireq->wscale_ok, &rcv_wscale,
> dst_metric(dst, RTAX_INITRWND));
> --
> 1.8.3.1
>
Thanks !
When net.ipv4.tcp_syncookies=1 and syn flood is happened,
cookie_v4_check or cookie_v6_check tries to redo what
tcp_v4_send_synack or tcp_v6_send_synack did,
rsk_window_clamp will be changed if SOCK_RCVBUF is set,
which will make rcv_wscale is different, the client
still operates with initial window scale and can overshot
granted window, the client use the initial scale but local
server use new scale to advertise window value, and session
work abnormally.
Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt")
Signed-off-by: Mao Wenan <[email protected]>
---
v4: change fixes tag format, and delay the actual call to
tcp_full_space().
v3: add local variable full_space, add fixes tag.
v2: fix for ipv6.
net/ipv4/syncookies.c | 8 +++++++-
net/ipv6/syncookies.c | 9 ++++++++-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 6ac473b..8784e1f 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -327,6 +327,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
struct inet_request_sock *ireq;
struct tcp_request_sock *treq;
struct tcp_sock *tp = tcp_sk(sk);
+ int full_space;
const struct tcphdr *th = tcp_hdr(skb);
__u32 cookie = ntohl(th->ack_seq) - 1;
struct sock *ret = sk;
@@ -427,8 +428,13 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
/* Try to redo what tcp_v4_send_synack did. */
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ full_space = tcp_full_space(sk);
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = full_space;
- tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
+ tcp_select_initial_window(sk, full_space, req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
ireq->wscale_ok, &rcv_wscale,
dst_metric(&rt->dst, RTAX_INITRWND));
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index e796a64..798ede8 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -132,6 +132,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
struct tcp_request_sock *treq;
struct ipv6_pinfo *np = inet6_sk(sk);
struct tcp_sock *tp = tcp_sk(sk);
+ int full_space;
const struct tcphdr *th = tcp_hdr(skb);
__u32 cookie = ntohl(th->ack_seq) - 1;
struct sock *ret = sk;
@@ -241,7 +242,13 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
}
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW);
- tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ full_space = tcp_full_space(sk);
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = full_space;
+
+ tcp_select_initial_window(sk, full_space, req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
ireq->wscale_ok, &rcv_wscale,
dst_metric(dst, RTAX_INITRWND));
--
1.8.3.1
On Mon, Nov 9, 2020 at 6:18 PM Mao Wenan <[email protected]> wrote:
>
> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
> cookie_v4_check or cookie_v6_check tries to redo what
> tcp_v4_send_synack or tcp_v6_send_synack did,
> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
> which will make rcv_wscale is different, the client
> still operates with initial window scale and can overshot
> granted window, the client use the initial scale but local
> server use new scale to advertise window value, and session
> work abnormally.
>
> Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt")
> Signed-off-by: Mao Wenan <[email protected]>
> ---
> v4: change fixes tag format, and delay the actual call to
> tcp_full_space().
> v3: add local variable full_space, add fixes tag.
> v2: fix for ipv6.
> net/ipv4/syncookies.c | 8 +++++++-
> net/ipv6/syncookies.c | 9 ++++++++-
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 6ac473b..8784e1f 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -327,6 +327,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
> struct inet_request_sock *ireq;
> struct tcp_request_sock *treq;
> struct tcp_sock *tp = tcp_sk(sk);
> + int full_space;
SGTM. although you could have avoided adding a variable breaking the almost
correct reverse Christmas tree that some of us prefer.
Something like this would look better :
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 6ac473b47f30d4d5e5e9455424b1a91d84e649ee..78af720f3e2c6dcdc7298178c5d2f02f0e425e04
100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -331,7 +331,7 @@ struct sock *cookie_v4_check(struct sock *sk,
struct sk_buff *skb)
__u32 cookie = ntohl(th->ack_seq) - 1;
struct sock *ret = sk;
struct request_sock *req;
- int mss;
+ int full_space, mss;
struct rtable *rt;
__u8 rcv_wscale;
struct flowi4 fl4;
When net.ipv4.tcp_syncookies=1 and syn flood is happened,
cookie_v4_check or cookie_v6_check tries to redo what
tcp_v4_send_synack or tcp_v6_send_synack did,
rsk_window_clamp will be changed if SOCK_RCVBUF is set,
which will make rcv_wscale is different, the client
still operates with initial window scale and can overshot
granted window, the client use the initial scale but local
server use new scale to advertise window value, and session
work abnormally.
Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt")
Signed-off-by: Mao Wenan <[email protected]>
---
v5: fix variable to adapat to Christmas tree format.
v4: change fixes tag format, and delay the actual call to
tcp_full_space().
v3: add local variable full_space, add fixes tag.
v2: fix for ipv6.
net/ipv4/syncookies.c | 9 +++++++--
net/ipv6/syncookies.c | 10 ++++++++--
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 6ac473b..00dc3f9 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -331,7 +331,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
__u32 cookie = ntohl(th->ack_seq) - 1;
struct sock *ret = sk;
struct request_sock *req;
- int mss;
+ int full_space, mss;
struct rtable *rt;
__u8 rcv_wscale;
struct flowi4 fl4;
@@ -427,8 +427,13 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
/* Try to redo what tcp_v4_send_synack did. */
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ full_space = tcp_full_space(sk);
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = full_space;
- tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
+ tcp_select_initial_window(sk, full_space, req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
ireq->wscale_ok, &rcv_wscale,
dst_metric(&rt->dst, RTAX_INITRWND));
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index e796a64..9b6cae1 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -136,7 +136,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
__u32 cookie = ntohl(th->ack_seq) - 1;
struct sock *ret = sk;
struct request_sock *req;
- int mss;
+ int full_space, mss;
struct dst_entry *dst;
__u8 rcv_wscale;
u32 tsoff = 0;
@@ -241,7 +241,13 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
}
req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW);
- tcp_select_initial_window(sk, tcp_full_space(sk), req->mss,
+ /* limit the window selection if the user enforce a smaller rx buffer */
+ full_space = tcp_full_space(sk);
+ if (sk->sk_userlocks & SOCK_RCVBUF_LOCK &&
+ (req->rsk_window_clamp > full_space || req->rsk_window_clamp == 0))
+ req->rsk_window_clamp = full_space;
+
+ tcp_select_initial_window(sk, full_space, req->mss,
&req->rsk_rcv_wnd, &req->rsk_window_clamp,
ireq->wscale_ok, &rcv_wscale,
dst_metric(dst, RTAX_INITRWND));
--
1.8.3.1
On Tue, Nov 10, 2020 at 1:16 AM Mao Wenan <[email protected]> wrote:
>
> When net.ipv4.tcp_syncookies=1 and syn flood is happened,
> cookie_v4_check or cookie_v6_check tries to redo what
> tcp_v4_send_synack or tcp_v6_send_synack did,
> rsk_window_clamp will be changed if SOCK_RCVBUF is set,
> which will make rcv_wscale is different, the client
> still operates with initial window scale and can overshot
> granted window, the client use the initial scale but local
> server use new scale to advertise window value, and session
> work abnormally.
>
> Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt")
> Signed-off-by: Mao Wenan <[email protected]>
Signed-off-by: Eric Dumazet <[email protected]>
Thanks !
On Tue, 10 Nov 2020 08:32:52 +0100 Eric Dumazet wrote:
> On Tue, Nov 10, 2020 at 1:16 AM Mao Wenan <[email protected]> wrote:
> > When net.ipv4.tcp_syncookies=1 and syn flood is happened,
> > cookie_v4_check or cookie_v6_check tries to redo what
> > tcp_v4_send_synack or tcp_v6_send_synack did,
> > rsk_window_clamp will be changed if SOCK_RCVBUF is set,
> > which will make rcv_wscale is different, the client
> > still operates with initial window scale and can overshot
> > granted window, the client use the initial scale but local
> > server use new scale to advertise window value, and session
> > work abnormally.
> >
> > Fixes: e88c64f0a425 ("tcp: allow effective reduction of TCP's rcv-buffer via setsockopt")
> > Signed-off-by: Mao Wenan <[email protected]>
>
> Signed-off-by: Eric Dumazet <[email protected]>
Applied, thanks!