Messages in this thread | | | Date | Mon, 23 May 2022 04:24:38 +0200 | From | Michal Kubecek <> | Subject | REGRESSION (?) (Re: [PATCH] net: af_key: add check for pfkey_broadcast in function pfkey_process) |
| |
On Tue, May 17, 2022 at 05:42:31PM +0800, Jiasheng Jiang wrote: > If skb_clone() returns null pointer, pfkey_broadcast() will > return error. > Therefore, it should be better to check the return value of > pfkey_broadcast() and return error if fails. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> > --- > net/key/af_key.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/key/af_key.c b/net/key/af_key.c > index fd51db3be91c..92e9d75dba2f 100644 > --- a/net/key/af_key.c > +++ b/net/key/af_key.c > @@ -2826,8 +2826,10 @@ static int pfkey_process(struct sock *sk, struct sk_buff *skb, const struct sadb > void *ext_hdrs[SADB_EXT_MAX]; > int err; > > - pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL, > - BROADCAST_PROMISC_ONLY, NULL, sock_net(sk)); > + err = pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL, > + BROADCAST_PROMISC_ONLY, NULL, sock_net(sk)); > + if (err) > + return err; > > memset(ext_hdrs, 0, sizeof(ext_hdrs)); > err = parse_exthdrs(skb, hdr, ext_hdrs);
After upgrading from 5.18-rc7 to 5.18 final, my racoon daemon refuses to start because it cannot find some algorithms (it says "aes"). I have not finished the debugging completely but this patch, mainline commit 4dc2a5a8f675 ("net: af_key: add check for pfkey_broadcast in function pfkey_process"), seems to be the most promising candidate.
As far as I can see, pfkey_broadcast() returns -ESRCH whenever it does not send the message to at least one registered listener. But this cannot happen here even if there were one as BROADCAST_PROMISC_ONLY flag makes pfkey_broadcast() skip the rest of the loop before err could be set:
sk_for_each_rcu(sk, &net_pfkey->table) { ... if (broadcast_flags != BROADCAST_ALL) { if (broadcast_flags & BROADCAST_PROMISC_ONLY) continue; if ((broadcast_flags & BROADCAST_REGISTERED) && !pfk->registered) continue; if (broadcast_flags & BROADCAST_ONE) continue; }
err2 = pfkey_broadcast_one(skb, GFP_ATOMIC, sk);
/* Error is cleared after successful sending to at least one * registered KM */ if ((broadcast_flags & BROADCAST_REGISTERED) && err) err = err2; }
and the only other option to change err from -ESRCH is
if (one_sk != NULL) err = pfkey_broadcast_one(skb, allocation, one_sk);
which cannot happen either as one_sk is null when called from pfkey_process().
So unless I missed something, bailing out on any non-zero return value in pfkey_process() is wrong without reworking the logic of pfkey_broadcast() return value first.
Michal [unhandled content-type:application/pgp-signature] | |