| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAL+tcoDVFtvg6+Kio9frU5W=2e2n7qrCJkitXUxNjsouAG+iGg@mail.gmail.com>
Date: Tue, 16 Apr 2024 20:24:31 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: dsahern@...nel.org, matttbe@...nel.org, martineau@...nel.org,
geliang@...nel.org, kuba@...nel.org, pabeni@...hat.com, davem@...emloft.net,
rostedt@...dmis.org, mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
atenart@...nel.org, mptcp@...ts.linux.dev, netdev@...r.kernel.org,
Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v4 4/6] tcp: support rstreason for passive reset
On Tue, Apr 16, 2024 at 3:45 PM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> On Tue, Apr 16, 2024 at 2:34 PM Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > On Thu, Apr 11, 2024 at 1:57 PM Jason Xing <kerneljasonxing@...il.com> wrote:
> > >
> > > From: Jason Xing <kernelxing@...cent.com>
> > >
> > > Reuse the dropreason logic to show the exact reason of tcp reset,
> > > so we don't need to implement those duplicated reset reasons.
> > > This patch replaces all the prior NOT_SPECIFIED reasons.
> > >
> > > Signed-off-by: Jason Xing <kernelxing@...cent.com>
> > > ---
> > > net/ipv4/tcp_ipv4.c | 8 ++++----
> > > net/ipv6/tcp_ipv6.c | 8 ++++----
> > > 2 files changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> > > index 441134aebc51..863397c2a47b 100644
> > > --- a/net/ipv4/tcp_ipv4.c
> > > +++ b/net/ipv4/tcp_ipv4.c
> > > @@ -1935,7 +1935,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
> > > return 0;
> > >
> > > reset:
> > > - tcp_v4_send_reset(rsk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > > + tcp_v4_send_reset(rsk, skb, (u32)reason);
> > > discard:
> > > kfree_skb_reason(skb, reason);
> > > /* Be careful here. If this function gets more complicated and
> > > @@ -2278,7 +2278,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> > > } else {
> > > drop_reason = tcp_child_process(sk, nsk, skb);
> > > if (drop_reason) {
> > > - tcp_v4_send_reset(nsk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > > + tcp_v4_send_reset(nsk, skb, (u32)drop_reason);
> >
> > Are all these casts really needed ?
>
> Not really. If without, the compiler wouldn't complain about it.
The truth is mptcp CI treats it as an error (see link[1]) when I
submitted the V5 patchset but my machine works well. I wonder whether
I should not remove all the casts or ignore the warnings?
[1]: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/8705084717/job/23874718134
net/ipv6/tcp_ipv6.c: In function 'tcp_v6_do_rcv':
4148 net/ipv6/tcp_ipv6.c:1683:36: error: implicit conversion from
'enum skb_drop_reason' to 'enum sk_rst_reason'
[-Werror=enum-conversion]
4149 1683 | tcp_v6_send_reset(sk, skb, reason);
4150 | ^~~~~~
4151 net/ipv6/tcp_ipv6.c: In function 'tcp_v6_rcv':
4152 net/ipv6/tcp_ipv6.c:1868:61: error: implicit conversion from
'enum skb_drop_reason' to 'enum sk_rst_reason'
[-Werror=enum-conversion]
4153 1868 | tcp_v6_send_reset(nsk, skb, drop_reason);
4154 | ^~~~~~~~~~~
4155 net/ipv6/tcp_ipv6.c:1945:46: error: implicit conversion from
'enum skb_drop_reason' to 'enum sk_rst_reason'
[-Werror=enum-conversion]
4156 1945 | tcp_v6_send_reset(NULL, skb, drop_reason);
4157 | ^~~~~~~~~~~
4158 net/ipv6/tcp_ipv6.c:2001:44: error: implicit conversion from
'enum skb_drop_reason' to 'enum sk_rst_reason'
[-Werror=enum-conversion]
4159 2001 | tcp_v6_send_reset(sk, skb, drop_reason);
Thanks,
Jason
>
> >
> > enum sk_rst_reason is not the same as u32 anyway ?
>
> I will remove the cast in the next version.
>
> Thanks,
> Jason
>
> >
> >
> >
> > > goto discard_and_relse;
> > > }
> > > sock_put(sk);
> > > @@ -2356,7 +2356,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> > > bad_packet:
> > > __TCP_INC_STATS(net, TCP_MIB_INERRS);
> > > } else {
> > > - tcp_v4_send_reset(NULL, skb, SK_RST_REASON_NOT_SPECIFIED);
> > > + tcp_v4_send_reset(NULL, skb, (u32)drop_reason);
> > > }
> > >
> > > discard_it:
> > > @@ -2407,7 +2407,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
> > > tcp_v4_timewait_ack(sk, skb);
> > > break;
> > > case TCP_TW_RST:
> > > - tcp_v4_send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > > + tcp_v4_send_reset(sk, skb, (u32)drop_reason);
> > > inet_twsk_deschedule_put(inet_twsk(sk));
> > > goto discard_it;
> > > case TCP_TW_SUCCESS:;
> > > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > > index 6cad32430a12..ba9d9ceb7e89 100644
> > > --- a/net/ipv6/tcp_ipv6.c
> > > +++ b/net/ipv6/tcp_ipv6.c
> > > @@ -1678,7 +1678,7 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
> > > return 0;
> > >
> > > reset:
> > > - tcp_v6_send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > > + tcp_v6_send_reset(sk, skb, (u32)reason);
> > > discard:
> > > if (opt_skb)
> > > __kfree_skb(opt_skb);
> > > @@ -1864,7 +1864,7 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
> > > } else {
> > > drop_reason = tcp_child_process(sk, nsk, skb);
> > > if (drop_reason) {
> > > - tcp_v6_send_reset(nsk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > > + tcp_v6_send_reset(nsk, skb, (u32)drop_reason);
> > > goto discard_and_relse;
> > > }
> > > sock_put(sk);
> > > @@ -1940,7 +1940,7 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
> > > bad_packet:
> > > __TCP_INC_STATS(net, TCP_MIB_INERRS);
> > > } else {
> > > - tcp_v6_send_reset(NULL, skb, SK_RST_REASON_NOT_SPECIFIED);
> > > + tcp_v6_send_reset(NULL, skb, (u32)drop_reason);
> > > }
> > >
> > > discard_it:
> > > @@ -1995,7 +1995,7 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
> > > tcp_v6_timewait_ack(sk, skb);
> > > break;
> > > case TCP_TW_RST:
> > > - tcp_v6_send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > > + tcp_v6_send_reset(sk, skb, (u32)drop_reason);
> > > inet_twsk_deschedule_put(inet_twsk(sk));
> > > goto discard_it;
> > > case TCP_TW_SUCCESS:
> > > --
> > > 2.37.3
> > >
Powered by blists - more mailing lists