Received-SPF: softfail (google.com: domain of transitioning linux-wireless-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Thu, 26 May 2022 11:35:26 +0200
From:   Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To:     Ryder Lee <ryder.lee@mediatek.com>
Cc:     linux-wireless@vger.kernel.org, Felix Fietkau <nbd@nbd.name>,
        Shayne Chen <shayne.chen@mediatek.com>,
        Evelyn Tsai <evelyn.tsai@mediatek.com>,
        linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] mac80211: check skb_shared in ieee80211_8023_xmit()
Message-ID: <Yo9J3j4mjD5ep6Da@lore-desk>
References: <1ef9b892cc93a36b1e62a6dda0e2e0a019f4e5f7.1653555361.git.ryder.lee@mediatek.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="Pe8cQTOWleRQHimU"
Content-Disposition: inline
In-Reply-To: <1ef9b892cc93a36b1e62a6dda0e2e0a019f4e5f7.1653555361.git.ryder.lee@mediatek.com>
Precedence: bulk


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

> Add missing skb_shared check into 802.3 path as 802.11 path does
> to prevent potential use-after-free from happening.
>=20
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  net/mac80211/tx.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>=20
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 0e4efc08c762..b026e746ac5b 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -4437,7 +4437,7 @@ static void ieee80211_8023_xmit(struct ieee80211_su=
b_if_data *sdata,
>  				struct net_device *dev, struct sta_info *sta,
>  				struct ieee80211_key *key, struct sk_buff *skb)
>  {
> -	struct ieee80211_tx_info *info =3D IEEE80211_SKB_CB(skb);
> +	struct ieee80211_tx_info *info;
>  	struct ieee80211_local *local =3D sdata->local;
>  	struct tid_ampdu_tx *tid_tx;
>  	u8 tid;
> @@ -4452,6 +4452,17 @@ static void ieee80211_8023_xmit(struct ieee80211_s=
ub_if_data *sdata,
>  	    test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state))
>  		goto out_free;
> =20
> +	if (skb_shared(skb)) {
> +		struct sk_buff *tmp_skb =3D skb;
> +
> +		skb =3D skb_clone(skb, GFP_ATOMIC);
> +		kfree_skb(tmp_skb);
> +
> +		if (!skb)
> +			return;
> +	}

I guess you can use skb_share_check() here instead.

Regards,
Lorenzo

> +
> +	info =3D IEEE80211_SKB_CB(skb);
>  	memset(info, 0, sizeof(*info));
> =20
>  	ieee80211_aggr_check(sdata, sta, skb);
> --=20
> 2.29.2
>=20

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

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

iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCYo9J3gAKCRA6cBh0uS2t
rFmCAQCaolYs75JOv0Zht8/vMW6lA2NyXUt6FnUf0Lsuh+7PagD8DiUDWY9LYMhF
WestUWb7Ak0vkSyywILqS3E5rRLOVQU=
=7M7Q
-----END PGP SIGNATURE-----

--Pe8cQTOWleRQHimU--