netdev - Re: [PATCH net-next v3 3/6] tcp: add dropreasons in tcp_rcv_state

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 13 Feb 2024 18:29:57 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, 
	dsahern@...nel.org, netdev@...r.kernel.org, 
	Jason Xing <kernelxing@...cent.com>, Kuniyuki Iwashima <kuniyu@...zon.com>
Subject: Re: [PATCH net-next v3 3/6] tcp: add dropreasons in tcp_rcv_state_process()

On Tue, Feb 13, 2024 at 5:35 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> >
> > Hi Eric, Kuniyuki
> >
> > Sorry, I should have checked tcp_conn_request() carefully last night.
> > Today, I checked tcp_conn_request() over and over again.
> >
> > I didn't find there is any chance to return a negative/positive value,
> > only 0. It means @acceptable is always true and it should never return
> > TCP_CONNREQNOTACCEPTABLE for TCP ipv4/6 protocol and never trigger a
> > reset in this way.
> >
>
> Then send a cleanup, thanks.
>
> A standalone patch is going to be simpler than reviewing a whole series.

I fear that I could misunderstand what you mean. I'm not that familiar
with how it works. Please enlighten me, thanks.

Are you saying I don't need to send a new version of the current patch
series, only send a patch after this series is applied?

A standalone patch goes like this based on this patch series:
diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 92513acca431..c059f7fc79f9 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -31,7 +31,6 @@
        FN(TCP_AOFAILURE)               \
        FN(SOCKET_BACKLOG)              \
        FN(TCP_FLAGS)                   \
-       FN(TCP_CONNREQNOTACCEPTABLE)                    \
        FN(TCP_ABORTONDATA)                     \
        FN(TCP_ZEROWINDOW)              \
        FN(TCP_OLD_DATA)                \
@@ -210,12 +209,6 @@ enum skb_drop_reason {
        SKB_DROP_REASON_SOCKET_BACKLOG,
        /** @SKB_DROP_REASON_TCP_FLAGS: TCP flags invalid */
        SKB_DROP_REASON_TCP_FLAGS,
-       /**
-        * @SKB_DROP_REASON_TCP_CONNREQNOTACCEPTABLE: connection request is not
-        * acceptable. This reason currently is a little bit obscure. It could
-        * be split into more specific reasons in the future.
-        */
-       SKB_DROP_REASON_TCP_CONNREQNOTACCEPTABLE,
        /**
         * @SKB_DROP_REASON_TCP_ABORTONDATA: abort on data, corresponding to
         * LINUX_MIB_TCPABORTONDATA
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d95f59f62742..023db3438b78 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6658,8 +6658,7 @@ tcp_rcv_state_process(struct sock *sk, struct
sk_buff *skb)
                        rcu_read_unlock();

                        if (!acceptable)
-                               /* This reason isn't clear. We can
refine it in the future */
-                               return SKB_DROP_REASON_TCP_CONNREQNOTACCEPTABLE;
+                               return 1;
                        consume_skb(skb);
                        return 0;
                }

If that is so, what is the status of the current patch?

Thanks,
Jason