All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Dinghao Liu <dinghao.liu@zju.edu.cn>
Cc: Alexander Aring <alex.aring@gmail.com>,
	Stefan Schmidt <stefan@datenfreihafen.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Marcel Holtmann <marcel@holtmann.org>,
	Harry Morris <harrymorris12@gmail.com>,
	linux-wpan@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [v2] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
Date: Tue, 26 Sep 2023 10:02:02 +0200	[thread overview]
Message-ID: <20230926100202.011ab841@xps-13> (raw)
In-Reply-To: <20230926032244.11560-1-dinghao.liu@zju.edu.cn>

Hi Dinghao,

dinghao.liu@zju.edu.cn wrote on Tue, 26 Sep 2023 11:22:44 +0800:

> If of_clk_add_provider() fails in ca8210_register_ext_clock(),
> it calls clk_unregister() to release priv->clk and returns an
> error. However, the caller ca8210_probe() then calls ca8210_remove(),
> where priv->clk is freed again in ca8210_unregister_ext_clock(). In
> this case, a use-after-free may happen in the second time we call
> clk_unregister().
> 
> Fix this by removing the first clk_unregister(). Also, priv->clk could
> be an error code on failure of clk_register_fixed_rate(). Use
> IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().
> 
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")

Missing Cc stable, this needs to be backported.

> Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> ---
> 
> Changelog:
> 
> v2: -Remove the first clk_unregister() instead of nulling priv->clk.
> ---
>  drivers/net/ieee802154/ca8210.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index aebb19f1b3a4..b35c6f59bd1a 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -2759,7 +2759,6 @@ static int ca8210_register_ext_clock(struct spi_device *spi)
>  	}
>  	ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
>  	if (ret) {
> -		clk_unregister(priv->clk);
>  		dev_crit(
>  			&spi->dev,
>  			"Failed to register external clock as clock provider\n"

I was hoping you would simplify this function a bit more.

> @@ -2780,7 +2779,7 @@ static void ca8210_unregister_ext_clock(struct spi_device *spi)
>  {
>  	struct ca8210_priv *priv = spi_get_drvdata(spi);
>  
> -	if (!priv->clk)
> +	if (IS_ERR_OR_NULL(priv->clk))
>  		return
>  
>  	of_clk_del_provider(spi->dev.of_node);

Alex, Stefan, who handles wpan and wpan/next this release?

Thanks,
Miquèl

  reply	other threads:[~2023-09-26  8:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-26  3:22 [PATCH] [v2] ieee802154: ca8210: Fix a potential UAF in ca8210_probe Dinghao Liu
2023-09-26  8:02 ` Miquel Raynal [this message]
2023-09-26  8:29   ` Stefan Schmidt
2023-09-26 16:40     ` Alexander Aring
2023-09-26 12:40   ` dinghao.liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230926100202.011ab841@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=alex.aring@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dinghao.liu@zju.edu.cn \
    --cc=edumazet@google.com \
    --cc=harrymorris12@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wpan@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stefan@datenfreihafen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.