Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 2 Mar 2021 23:56:46 +0900
From:   William Breathitt Gray <vilhelm.gray@gmail.com>
To:     Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Cc:     jic23@kernel.org, david@lechnology.com,
        alexandre.torgue@foss.st.com, mcoquelin.stm32@gmail.com,
        olivier.moysan@foss.st.com, linux-iio@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] counter: stm32-timer-cnt: fix ceiling write max value
Message-ID: <YD5SLrdttn+95M7N@shinobu>
References: <1614696235-24088-1-git-send-email-fabrice.gasnier@foss.st.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="wvX72FUvr+D6FcPx"
Content-Disposition: inline
In-Reply-To: <1614696235-24088-1-git-send-email-fabrice.gasnier@foss.st.com>
Precedence: bulk


--wvX72FUvr+D6FcPx
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 02, 2021 at 03:43:55PM +0100, Fabrice Gasnier wrote:
> The ceiling value isn't checked before writing it into registers. The user
> could write a value higher than the counter resolution (e.g. 16 or 32 bits
> indicated by max_arr). This makes most significant bits to be truncated.
> Fix it by checking the max_arr to report a range error [1] to the user.
>=20
> Fixes: ad29937e206f ("counter: Add STM32 Timer quadrature encoder")
>=20
> [1] https://lkml.org/lkml/2021/2/12/358
>=20
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com>

Side question: if priv->ceiling is tracking the current ceiling
configuration, would it make sense to change stm32_count_ceiling_read()
to print the value of priv->ceiling instead of doing a regmap_read()
call?

> ---
>  drivers/counter/stm32-timer-cnt.c | 5 +++++
>  1 file changed, 5 insertions(+)
>=20
> diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-ti=
mer-cnt.c
> index ef2a974..2cf0c05 100644
> --- a/drivers/counter/stm32-timer-cnt.c
> +++ b/drivers/counter/stm32-timer-cnt.c
> @@ -32,6 +32,7 @@ struct stm32_timer_cnt {
>  	struct regmap *regmap;
>  	struct clk *clk;
>  	u32 ceiling;
> +	u32 max_arr;
>  	bool enabled;
>  	struct stm32_timer_regs bak;
>  };
> @@ -185,6 +186,9 @@ static ssize_t stm32_count_ceiling_write(struct count=
er_device *counter,
>  	if (ret)
>  		return ret;
> =20
> +	if (ceiling > priv->max_arr)
> +		return -ERANGE;
> +
>  	/* TIMx_ARR register shouldn't be buffered (ARPE=3D0) */
>  	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, 0);
>  	regmap_write(priv->regmap, TIM_ARR, ceiling);
> @@ -360,6 +364,7 @@ static int stm32_timer_cnt_probe(struct platform_devi=
ce *pdev)
>  	priv->regmap =3D ddata->regmap;
>  	priv->clk =3D ddata->clk;
>  	priv->ceiling =3D ddata->max_arr;
> +	priv->max_arr =3D ddata->max_arr;
> =20
>  	priv->counter.name =3D dev_name(dev);
>  	priv->counter.parent =3D dev;
> --=20
> 2.7.4
>=20

--wvX72FUvr+D6FcPx
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEk5I4PDJ2w1cDf/bghvpINdm7VJIFAmA+UggACgkQhvpINdm7
VJJt3g/+OVZcvByeL/DXkUu5tzYQxQ5xzzfViSO16w54567c2tGJuB8gBEFaK9eG
Q5EzOeM9YaL982A/9OtqQup3CAJVkLizpy65aH1jDlPasA8JKKoelTfd+mAo58Vu
hiwG3DXxwOCZNB7b2/+RaKcYyayviVsKgrl8mHUQ1SY81kM3/cdzSLSnAoxkfD8o
7w1CdXnOxDvvTD6rwcbc4izhSSxgHzvaGHFpKwwYa3gDpmCY44rDKsgXc0CL6F2B
1JRDgvIePdqIStdC8VXcGkUwmlrEVwfSqFgbAtadWuwMSR293AR1PCayvsA4yWyo
49fMudR81RlG5aFyYoOWbAvoKpiq81LlGQspGWFcND4mdbDR7qvEeuZO//0wbTxk
5bNTj/VrQeSyNb7HmDFzOaKwF3waAnYpwZyOcUKVwzVNyAJkRq4nCPn29OKHE3mC
/Pbun8nAcbF0BP5R6QzX++aELtXzuArqJHOCDudTDQas0i7BNH0/+1KNKvisLTbk
6ftDbo0p3M90RUks08LLVpDzrAUGpyWeq7+VmWj1jHj22759PtbkEd/dxXKMkcQk
OwD4+2zRN4yq4J2U8oiofAprHHgidhAXPB+Qvy4GXP1SewZl3q+cXCNn20Se39Xw
uEDiJrNEX2zfbyRHx1V1zjqBlXFvCKIt62uqb8FBMmexuC5fmbc=
=M2Bd
-----END PGP SIGNATURE-----

--wvX72FUvr+D6FcPx--