Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Mon, 23 May 2022 04:24:38 +0200
From:   Michal Kubecek <mkubecek@suse.cz>
To:     Jiasheng Jiang <jiasheng@iscas.ac.cn>
Cc:     steffen.klassert@secunet.com, herbert@gondor.apana.org.au,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: REGRESSION (?) (Re: [PATCH] net: af_key: add check for
 pfkey_broadcast in function pfkey_process)
Message-ID: <20220523022438.ofhehjievu2alj3h@lion.mk-sys.cz>
References: <20220517094231.414168-1-jiasheng@iscas.ac.cn>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="g42tgft3y3hx52j4"
Content-Disposition: inline
In-Reply-To: <20220517094231.414168-1-jiasheng@iscas.ac.cn>
Precedence: bulk


--g42tgft3y3hx52j4
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
>=20
> 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(-)
>=20
> 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 s=
k_buff *skb, const struct sadb
>  	void *ext_hdrs[SADB_EXT_MAX];
>  	int err;
> =20
> -	pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL,
> -			BROADCAST_PROMISC_ONLY, NULL, sock_net(sk));
> +	err =3D pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL,
> +			      BROADCAST_PROMISC_ONLY, NULL, sock_net(sk));
> +	if (err)
> +		return err;
> =20
>  	memset(ext_hdrs, 0, sizeof(ext_hdrs));
>  	err =3D 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) {
=2E..
		if (broadcast_flags !=3D BROADCAST_ALL) {
			if (broadcast_flags & BROADCAST_PROMISC_ONLY)
				continue;
			if ((broadcast_flags & BROADCAST_REGISTERED) &&
			    !pfk->registered)
				continue;
			if (broadcast_flags & BROADCAST_ONE)
				continue;
		}

		err2 =3D 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 =3D err2;
	}

and the only other option to change err from -ESRCH is

	if (one_sk !=3D NULL)
		err =3D 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.=20

Michal

--g42tgft3y3hx52j4
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCAAdFiEEWN3j3bieVmp26mKO538sG/LRdpUFAmKK8GAACgkQ538sG/LR
dpUHTAf9ERCtb+WT/nqUex+gV8YmPze/VHGfpivZX06w3CEDBI0VUtt7rGJDDGGu
3Ca4WGfZRlLQk5da3ysbgZA7iTBTIsbYrnxHzNXjiBA4/w8Nuf1Rn/2cpk4Kyafb
FvqLfFCxeF6HXH+rtsFIe7nQOWewY5shqOOOhRWeq8z8kQou9RWoM/Pg/IVmhuyz
j4hnuDsar1Dot9d1Fxxq5tP8xzUqCm6t5Qya+3ZE9nhL1ACaxTrsg+pOZmb5tBg8
ymtQra26k/hfii9vX8cBxb85p8kigpsbPtDPl3WEXiwgO+utdu+gQ7L7WQ9e4DGH
Vsrg7eEKdMZby1lkM16WEFIduzJmrw==
=Eg5V
-----END PGP SIGNATURE-----

--g42tgft3y3hx52j4--