linux-kernel - Re: [PATCH v8 01/26] tcp: authopt: Initial support and key management

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: <589e17df-e321-c8ad-5360-e286c10cb1a3@gmail.com>
Date:   Thu, 8 Sep 2022 13:47:23 +0300
From:   Leonard Crestez <cdleonard@...il.com>
To:     Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     Francesco Ruggeri <fruggeri@...sta.com>,
        Salam Noureddine <noureddine@...sta.com>,
        Philip Paeps <philip@...uble.is>,
        Shuah Khan <shuah@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Kuniyuki Iwashima <kuniyu@...zon.co.jp>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Yuchung Cheng <ycheng@...gle.com>,
        Mat Martineau <mathew.j.martineau@...ux.intel.com>,
        Christoph Paasch <cpaasch@...le.com>,
        Ivan Delalande <colona@...sta.com>,
        Caowangbao <caowangbao@...wei.com>,
        Priyaranjan Jha <priyarjha@...gle.com>, netdev@...r.kernel.org,
        linux-crypto@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org, Dmitry Safonov <0x7f454c46@...il.com>
Subject: Re: [PATCH v8 01/26] tcp: authopt: Initial support and key management

On 9/8/22 09:35, Paolo Abeni wrote:
> On Mon, 2022-09-05 at 10:05 +0300, Leonard Crestez wrote:
> [...]
>> diff --git a/net/ipv4/tcp_authopt.c b/net/ipv4/tcp_authopt.c
>> new file mode 100644
>> index 000000000000..d38e9c89c89d
>> --- /dev/null
>> +++ b/net/ipv4/tcp_authopt.c
>> @@ -0,0 +1,317 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +#include <net/tcp_authopt.h>
>> +#include <net/ipv6.h>
>> +#include <net/tcp.h>
>> +#include <linux/kref.h>
>> +
>> +/* This is enabled when first struct tcp_authopt_info is allocated and never released */
>> +DEFINE_STATIC_KEY_FALSE(tcp_authopt_needed_key);
>> +EXPORT_SYMBOL(tcp_authopt_needed_key);
>> +
>> +static inline struct netns_tcp_authopt *sock_net_tcp_authopt(const struct sock *sk)
>> +{
>> +	return &sock_net(sk)->tcp_authopt;
>> +}
> 
> Please have a look at PW report for this series, there are a bunch of
> issues to be addressed, e.g. above 'static inline' should be just
> 'static'

What is a "PW report"? I can't find any info about this.

>> +static void tcp_authopt_key_release_kref(struct kref *ref)
>> +{
>> +	struct tcp_authopt_key_info *key = container_of(ref, struct tcp_authopt_key_info, ref);
>> +
>> +	kfree_rcu(key, rcu);
>> +}
>> +
>> +static void tcp_authopt_key_put(struct tcp_authopt_key_info *key)
>> +{
>> +	if (key)
>> +		kref_put(&key->ref, tcp_authopt_key_release_kref);
>> +}
>> +
>> +static void tcp_authopt_key_del(struct netns_tcp_authopt *net,
>> +				struct tcp_authopt_key_info *key)
>> +{
>> +	lockdep_assert_held(&net->mutex);
>> +	hlist_del_rcu(&key->node);
>> +	key->flags |= TCP_AUTHOPT_KEY_DEL;
>> +	kref_put(&key->ref, tcp_authopt_key_release_kref);
>> +}
>> +
>> +/* Free info and keys.
>> + * Don't touch tp->authopt_info, it might not even be assigned yes.
>> + */
>> +void tcp_authopt_free(struct sock *sk, struct tcp_authopt_info *info)
> 
> this need to be 'static'.

Tried this and it's later called from tcp_twsk_destructor.

> I'm sorry to bring the next topic this late (If already discussed, I
> missed that point), is possible to split this series in smaller chunks?

It's already 26 patches and 3675 added lines, less that 150 lines per 
patch seems reasonable?

The split is already somewhat artificial, for example there are patches 
that "add crypto" without actually using it because then it would be too 
large.

Some features could be dropped for later in order to make this smaller, 
for example TCP_REPAIR doesn't have many usecases. Features like 
prefixlen, vrf binding and ipv4-mapped-ipv6 were explicitly requested by 
maintainers so I included them as separate patches in the main series.

--
Regards,
Leonard