net/tcp: Add missing lockdep annotations for TCP-AO hlist traversals
BugLink: https://bugs.launchpad.net/bugs/2102118 [ Upstream commit 6b2d11e2d8fc130df4708be0b6b53fd3e6b54cf6 ] Under CONFIG_PROVE_RCU_LIST + CONFIG_RCU_EXPERT hlist_for_each_entry_rcu() provides very helpful splats, which help to find possible issues. I missed CONFIG_RCU_EXPERT=y in my testing config the same as described in a3e4bf7f9675 ("configs/debug: make sure PROVE_RCU_LIST=y takes effect"). The fix itself is trivial: add the very same lockdep annotations as were used to dereference ao_info from the socket. Reported-by: Jakub Kicinski <kuba@kernel.org> Closes: https://lore.kernel.org/netdev/20241028152645.35a8be66@kernel.org/ Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20241030-tcp-ao-hlist-lockdep-annotate-v1-1-bf641a64d7c6@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Koichiro Den <koichiro.den@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
This commit is contained in:
committed by
Stefan Bader
parent
97c0ee1614
commit
351c839cde
@@ -215,7 +215,8 @@ int tcp_ao_hash_skb(unsigned short int family,
|
||||
const u8 *tkey, int hash_offset, u32 sne);
|
||||
int tcp_parse_ao(struct sock *sk, int cmd, unsigned short int family,
|
||||
sockptr_t optval, int optlen);
|
||||
struct tcp_ao_key *tcp_ao_established_key(struct tcp_ao_info *ao,
|
||||
struct tcp_ao_key *tcp_ao_established_key(const struct sock *sk,
|
||||
struct tcp_ao_info *ao,
|
||||
int sndid, int rcvid);
|
||||
int tcp_ao_copy_all_matching(const struct sock *sk, struct sock *newsk,
|
||||
struct request_sock *req, struct sk_buff *skb,
|
||||
|
||||
+23
-19
@@ -108,12 +108,13 @@ bool tcp_ao_ignore_icmp(const struct sock *sk, int family, int type, int code)
|
||||
* it's known that the keys in ao_info are matching peer's
|
||||
* family/address/VRF/etc.
|
||||
*/
|
||||
struct tcp_ao_key *tcp_ao_established_key(struct tcp_ao_info *ao,
|
||||
struct tcp_ao_key *tcp_ao_established_key(const struct sock *sk,
|
||||
struct tcp_ao_info *ao,
|
||||
int sndid, int rcvid)
|
||||
{
|
||||
struct tcp_ao_key *key;
|
||||
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node) {
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk)) {
|
||||
if ((sndid >= 0 && key->sndid != sndid) ||
|
||||
(rcvid >= 0 && key->rcvid != rcvid))
|
||||
continue;
|
||||
@@ -204,7 +205,7 @@ static struct tcp_ao_key *__tcp_ao_do_lookup(const struct sock *sk, int l3index,
|
||||
if (!ao)
|
||||
return NULL;
|
||||
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node) {
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk)) {
|
||||
u8 prefixlen = min(prefix, key->prefixlen);
|
||||
|
||||
if (!tcp_ao_key_cmp(key, l3index, addr, prefixlen,
|
||||
@@ -792,7 +793,7 @@ int tcp_ao_prepare_reset(const struct sock *sk, struct sk_buff *skb,
|
||||
if (!ao_info)
|
||||
return -ENOENT;
|
||||
|
||||
*key = tcp_ao_established_key(ao_info, aoh->rnext_keyid, -1);
|
||||
*key = tcp_ao_established_key(sk, ao_info, aoh->rnext_keyid, -1);
|
||||
if (!*key)
|
||||
return -ENOENT;
|
||||
*traffic_key = snd_other_key(*key);
|
||||
@@ -978,7 +979,7 @@ tcp_inbound_ao_hash(struct sock *sk, const struct sk_buff *skb,
|
||||
*/
|
||||
key = READ_ONCE(info->rnext_key);
|
||||
if (key->rcvid != aoh->keyid) {
|
||||
key = tcp_ao_established_key(info, -1, aoh->keyid);
|
||||
key = tcp_ao_established_key(sk, info, -1, aoh->keyid);
|
||||
if (!key)
|
||||
goto key_not_found;
|
||||
}
|
||||
@@ -999,7 +1000,7 @@ tcp_inbound_ao_hash(struct sock *sk, const struct sk_buff *skb,
|
||||
/* Key rotation: the peer asks us to use new key (RNext) */
|
||||
if (unlikely(aoh->rnext_keyid != current_key->sndid)) {
|
||||
/* If the key is not found we do nothing. */
|
||||
key = tcp_ao_established_key(info, aoh->rnext_keyid, -1);
|
||||
key = tcp_ao_established_key(sk, info, aoh->rnext_keyid, -1);
|
||||
if (key)
|
||||
/* pairs with tcp_ao_del_cmd */
|
||||
WRITE_ONCE(info->current_key, key);
|
||||
@@ -1159,7 +1160,7 @@ void tcp_ao_established(struct sock *sk)
|
||||
if (!ao)
|
||||
return;
|
||||
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node)
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk))
|
||||
tcp_ao_cache_traffic_keys(sk, ao, key);
|
||||
}
|
||||
|
||||
@@ -1176,7 +1177,7 @@ void tcp_ao_finish_connect(struct sock *sk, struct sk_buff *skb)
|
||||
WRITE_ONCE(ao->risn, tcp_hdr(skb)->seq);
|
||||
ao->rcv_sne = 0;
|
||||
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node)
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk))
|
||||
tcp_ao_cache_traffic_keys(sk, ao, key);
|
||||
}
|
||||
|
||||
@@ -1252,14 +1253,14 @@ int tcp_ao_copy_all_matching(const struct sock *sk, struct sock *newsk,
|
||||
key_head = rcu_dereference(hlist_first_rcu(&new_ao->head));
|
||||
first_key = hlist_entry_safe(key_head, struct tcp_ao_key, node);
|
||||
|
||||
key = tcp_ao_established_key(new_ao, tcp_rsk(req)->ao_keyid, -1);
|
||||
key = tcp_ao_established_key(req_to_sk(req), new_ao, tcp_rsk(req)->ao_keyid, -1);
|
||||
if (key)
|
||||
new_ao->current_key = key;
|
||||
else
|
||||
new_ao->current_key = first_key;
|
||||
|
||||
/* set rnext_key */
|
||||
key = tcp_ao_established_key(new_ao, -1, tcp_rsk(req)->ao_rcv_next);
|
||||
key = tcp_ao_established_key(req_to_sk(req), new_ao, -1, tcp_rsk(req)->ao_rcv_next);
|
||||
if (key)
|
||||
new_ao->rnext_key = key;
|
||||
else
|
||||
@@ -1853,12 +1854,12 @@ static int tcp_ao_del_cmd(struct sock *sk, unsigned short int family,
|
||||
* if there's any.
|
||||
*/
|
||||
if (cmd.set_current) {
|
||||
new_current = tcp_ao_established_key(ao_info, cmd.current_key, -1);
|
||||
new_current = tcp_ao_established_key(sk, ao_info, cmd.current_key, -1);
|
||||
if (!new_current)
|
||||
return -ENOENT;
|
||||
}
|
||||
if (cmd.set_rnext) {
|
||||
new_rnext = tcp_ao_established_key(ao_info, -1, cmd.rnext);
|
||||
new_rnext = tcp_ao_established_key(sk, ao_info, -1, cmd.rnext);
|
||||
if (!new_rnext)
|
||||
return -ENOENT;
|
||||
}
|
||||
@@ -1898,7 +1899,8 @@ static int tcp_ao_del_cmd(struct sock *sk, unsigned short int family,
|
||||
* "It is presumed that an MKT affecting a particular
|
||||
* connection cannot be destroyed during an active connection"
|
||||
*/
|
||||
hlist_for_each_entry_rcu(key, &ao_info->head, node) {
|
||||
hlist_for_each_entry_rcu(key, &ao_info->head, node,
|
||||
lockdep_sock_is_held(sk)) {
|
||||
if (cmd.sndid != key->sndid ||
|
||||
cmd.rcvid != key->rcvid)
|
||||
continue;
|
||||
@@ -1996,14 +1998,14 @@ static int tcp_ao_info_cmd(struct sock *sk, unsigned short int family,
|
||||
* if there's any.
|
||||
*/
|
||||
if (cmd.set_current) {
|
||||
new_current = tcp_ao_established_key(ao_info, cmd.current_key, -1);
|
||||
new_current = tcp_ao_established_key(sk, ao_info, cmd.current_key, -1);
|
||||
if (!new_current) {
|
||||
err = -ENOENT;
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
if (cmd.set_rnext) {
|
||||
new_rnext = tcp_ao_established_key(ao_info, -1, cmd.rnext);
|
||||
new_rnext = tcp_ao_established_key(sk, ao_info, -1, cmd.rnext);
|
||||
if (!new_rnext) {
|
||||
err = -ENOENT;
|
||||
goto out;
|
||||
@@ -2097,7 +2099,8 @@ int tcp_v4_parse_ao(struct sock *sk, int cmd, sockptr_t optval, int optlen)
|
||||
* The layout of the fields in the user and kernel structures is expected to
|
||||
* be the same (including in the 32bit vs 64bit case).
|
||||
*/
|
||||
static int tcp_ao_copy_mkts_to_user(struct tcp_ao_info *ao_info,
|
||||
static int tcp_ao_copy_mkts_to_user(const struct sock *sk,
|
||||
struct tcp_ao_info *ao_info,
|
||||
sockptr_t optval, sockptr_t optlen)
|
||||
{
|
||||
struct tcp_ao_getsockopt opt_in, opt_out;
|
||||
@@ -2225,7 +2228,8 @@ static int tcp_ao_copy_mkts_to_user(struct tcp_ao_info *ao_info,
|
||||
/* May change in RX, while we're dumping, pre-fetch it */
|
||||
current_key = READ_ONCE(ao_info->current_key);
|
||||
|
||||
hlist_for_each_entry_rcu(key, &ao_info->head, node) {
|
||||
hlist_for_each_entry_rcu(key, &ao_info->head, node,
|
||||
lockdep_sock_is_held(sk)) {
|
||||
if (opt_in.get_all)
|
||||
goto match;
|
||||
|
||||
@@ -2305,7 +2309,7 @@ int tcp_ao_get_mkts(struct sock *sk, sockptr_t optval, sockptr_t optlen)
|
||||
if (!ao_info)
|
||||
return -ENOENT;
|
||||
|
||||
return tcp_ao_copy_mkts_to_user(ao_info, optval, optlen);
|
||||
return tcp_ao_copy_mkts_to_user(sk, ao_info, optval, optlen);
|
||||
}
|
||||
|
||||
int tcp_ao_get_sock_info(struct sock *sk, sockptr_t optval, sockptr_t optlen)
|
||||
@@ -2392,7 +2396,7 @@ int tcp_ao_set_repair(struct sock *sk, sockptr_t optval, unsigned int optlen)
|
||||
WRITE_ONCE(ao->snd_sne, cmd.snd_sne);
|
||||
WRITE_ONCE(ao->rcv_sne, cmd.rcv_sne);
|
||||
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node)
|
||||
hlist_for_each_entry_rcu(key, &ao->head, node, lockdep_sock_is_held(sk))
|
||||
tcp_ao_cache_traffic_keys(sk, ao, key);
|
||||
|
||||
return 0;
|
||||
|
||||
+2
-1
@@ -1044,7 +1044,8 @@ static void tcp_v4_timewait_ack(struct sock *sk, struct sk_buff *skb)
|
||||
}
|
||||
|
||||
if (aoh)
|
||||
key.ao_key = tcp_ao_established_key(ao_info, aoh->rnext_keyid, -1);
|
||||
key.ao_key = tcp_ao_established_key(sk, ao_info,
|
||||
aoh->rnext_keyid, -1);
|
||||
}
|
||||
}
|
||||
if (key.ao_key) {
|
||||
|
||||
+2
-2
@@ -1162,8 +1162,8 @@ static void tcp_v6_timewait_ack(struct sock *sk, struct sk_buff *skb)
|
||||
if (tcp_parse_auth_options(tcp_hdr(skb), NULL, &aoh))
|
||||
goto out;
|
||||
if (aoh)
|
||||
key.ao_key = tcp_ao_established_key(ao_info,
|
||||
aoh->rnext_keyid, -1);
|
||||
key.ao_key = tcp_ao_established_key(sk, ao_info,
|
||||
aoh->rnext_keyid, -1);
|
||||
}
|
||||
}
|
||||
if (key.ao_key) {
|
||||
|
||||
Reference in New Issue
Block a user