netdev - Re: [PATCH] net/tcp: track all ipv4/tcp state transition in tcp_set

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]

Message-Id: <20171108.155112.1483404763325763621.davem@davemloft.net>
Date:   Wed, 08 Nov 2017 15:51:12 +0900 (KST)
From:   David Miller <davem@...emloft.net>
To:     laoar.shao@...il.com
Cc:     kuznet@....inr.ac.ru, yoshfuji@...ux-ipv6.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net/tcp: track all ipv4/tcp state transition in
 tcp_set_state

From: Yafang Shao <laoar.shao@...il.com>
Date: Tue,  7 Nov 2017 18:36:28 +0800

> When I hooked the function tcp_set_state with kprobe to track the ipv4/tcp
> state transistion, I found state transition from TCP_LISTEN to TCP_SYN_RECV
> is missed.
> 
> I think it is better to use the helper to do state transition instead of
> assigning the state to sk_state directly.
> Then we can monitor the whole tcp lifespans with kprobe or ftrace easily.
> 
> Signed-off-by: Yafang Shao <laoar.shao@...il.com>

This is really heavy handed and excessive for these cases.

They don't have to handle any of the issues dealt with in
tcp_set_state().

I would prefer if you made a special helper to net/tcp.h which did:

static inline void __tcp_set_state(struct sock *sk, int state)
{
	trace_tcp_set_state(sk, sk->sk_state, state);
	sk->sk_state = state;
}