Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 17 Jun 2021 15:52:21 +0200
From:   Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To:     sean.wang@mediatek.com
Cc:     nbd@nbd.name, Soul.Huang@mediatek.com, YN.Chen@mediatek.com,
        Leon.Yen@mediatek.com, Eric-SY.Chang@mediatek.com,
        Deren.Wu@mediatek.com, km.lin@mediatek.com,
        robin.chiu@mediatek.com, ch.yeh@mediatek.com,
        posh.sun@mediatek.com, Eric.Liang@mediatek.com,
        Stella.Chang@mediatek.com, jemele@google.com, yenlinlai@google.com,
        linux-wireless@vger.kernel.org, linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] mt76: mt7921: fix the coredump is being truncated
Message-ID: <YMtTlS3Nh4uGnJvq@lore-desk>
References: <2ebb95099102ffe62c0fa8aedd95d7e77b5ca385.1623884856.git.objelf@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="/rucmuLPAqyrwX+6"
Content-Disposition: inline
In-Reply-To: <2ebb95099102ffe62c0fa8aedd95d7e77b5ca385.1623884856.git.objelf@gmail.com>
Precedence: bulk


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

> From: Sean Wang <sean.wang@mediatek.com>
>=20
> Fix the maximum size of the coredump generated with current mt7921
> firmware. Otherwise, a truncated coredump would be reported to userland
> via dev_coredumpv.
>=20
> Also, there is an additional error handling enhanced in the patch to avoid
> the possible invalid buffer access when the system failed to create the
> buffer to hold the coredump.
>=20
> Fixes: 0da3c795d07b ("mt76: mt7921: add coredump support")
> Co-developed-by: YN Chen <YN.Chen@mediatek.com>
> Signed-off-by: YN Chen <YN.Chen@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt76_connac.h | 2 +-
>  drivers/net/wireless/mediatek/mt76/mt7921/mac.c  | 9 ++++++---
>  2 files changed, 7 insertions(+), 4 deletions(-)
>=20
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac.h b/drivers/n=
et/wireless/mediatek/mt76/mt76_connac.h
> index 9b3f8d22f17e..d93ab1ece8ae 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac.h
> @@ -13,7 +13,7 @@
>  #define MT76_CONNAC_MAX_SCAN_MATCH		16
> =20
>  #define MT76_CONNAC_COREDUMP_TIMEOUT		(HZ / 20)
> -#define MT76_CONNAC_COREDUMP_SZ			(128 * 1024)
> +#define MT76_CONNAC_COREDUMP_SZ			(1300 * 1024)
> =20
>  enum {
>  	CMD_CBW_20MHZ =3D IEEE80211_STA_RX_BW_20,
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c b/drivers/ne=
t/wireless/mediatek/mt76/mt7921/mac.c
> index fb4de73df701..905dddbfbb0b 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
> @@ -1557,7 +1557,7 @@ void mt7921_coredump_work(struct work_struct *work)
>  			break;
> =20
>  		skb_pull(skb, sizeof(struct mt7921_mcu_rxd));
> -		if (data + skb->len - dump > MT76_CONNAC_COREDUMP_SZ) {
> +		if (!dump || data + skb->len - dump > MT76_CONNAC_COREDUMP_SZ) {

why not just return if dump allocation fails? Doing so we will reset the de=
vice
even if dump is NULL, do you think it is a real use-case?

Regards,
Lorenzo

>  			dev_kfree_skb(skb);
>  			continue;
>  		}
> @@ -1567,7 +1567,10 @@ void mt7921_coredump_work(struct work_struct *work)
> =20
>  		dev_kfree_skb(skb);
>  	}
> -	dev_coredumpv(dev->mt76.dev, dump, MT76_CONNAC_COREDUMP_SZ,
> -		      GFP_KERNEL);
> +
> +	if (dump)
> +		dev_coredumpv(dev->mt76.dev, dump, MT76_CONNAC_COREDUMP_SZ,
> +			      GFP_KERNEL);
> +
>  	mt7921_reset(&dev->mt76);
>  }
> --=20
> 2.25.1
>=20

--/rucmuLPAqyrwX+6
Content-Type: application/pgp-signature; name="signature.asc"

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

iHUEABYIAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCYMtTkgAKCRA6cBh0uS2t
rIHjAQCqNVvge4z+NsfUu91P1Cey8w++PXYbKlGOEW8i9n++kgEAjU5Q6TPVUx/n
1Wzjj3kkXpDbJSvZuKl+IZJbOnRKzwo=
=rDOE
-----END PGP SIGNATURE-----

--/rucmuLPAqyrwX+6--