Re: [PATCH v3 3/3] mux: Avoid use-after-free of args.fwnode in mux_get()

[Alvin Šipraga]

On Tue, Sep 29, 2026 at 10:51:48PM +0200, Fabio Forni via B4 Relay wrote:
> From: Fabio Forni <development@xxxxxxxxxx>
> 
> fwnode_handle_put(args.fwnode) was called right after
> mux_chip_find_by_fwnode(), but it was too early because the error
> handling code below would pass args.fwnode to dev_err().
> Let's move all freeing functions to the bottom of mux_get() to avoid
> use-after-free bugs.
> 
> Signed-off-by: Fabio Forni <development@xxxxxxxxxx>

I meant to suggest that you apply this fix before your fwnode patch, so
that it can be applied to the stable trees. Since you put the fix
afterwards, it either needs backporting, or the fwnode patch needs to be
carried too.

You might also want a Fixes: tag for it to actually get picked up for
stable.

Up to Peter though - it's a minor bug and the kernel has tons of such
refcounting bloopers. As far as the code is concerned it looks fine
(minor nit below). Thanks!

Reviewed-by: Alvin Šipraga <alvin.sipraga@xxxxxxxxxx>

> ---
>  drivers/mux/core.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index d5121772c483..8bf8c79bc634 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -545,6 +545,9 @@ static struct mux_chip *mux_chip_find_by_fwnode(struct fwnode_handle *fwnode)
>   * @optional: Whether to return NULL and silence errors when mux doesn't exist.
>   * @node: the device nodes, use dev's fwnode if it is NULL.
>   *
> + * When a mux-control is found, it is the caller's responsibility to call
> + * mux_control_put() on it when it is no longer needed.
> + *
>   * Return: Pointer to the mux-control on success, an ERR_PTR with a negative
>   * errno on error, or NULL if optional is true and mux doesn't exist.
>   */
> @@ -597,9 +600,10 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  	}
>  
>  	mux_chip = mux_chip_find_by_fwnode(args.fwnode);
> -	fwnode_handle_put(args.fwnode);
> -	if (!mux_chip)
> -		return ERR_PTR(-EPROBE_DEFER);
> +	if (!mux_chip) {
> +		ret = -EPROBE_DEFER;
> +		goto end;
> +	}
>  
>  	controller = 0;
>  	if (state) {
> @@ -607,8 +611,8 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  		    (args.nargs < 2 && mux_chip->controllers > 1)) {
>  			dev_err(dev, "%pfw: wrong #mux-state-cells for %pfw\n",
>  				fwnode, args.fwnode);
> -			put_device(&mux_chip->dev);
> -			return ERR_PTR(-EINVAL);
> +			ret = -EINVAL;
> +			goto end;
>  		}
>  
>  		if (args.nargs == 2) {
> @@ -623,8 +627,8 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  		    (!args.nargs && mux_chip->controllers > 1)) {
>  			dev_err(dev, "%pfw: wrong #mux-control-cells for %pfw\n",
>  				fwnode, args.fwnode);
> -			put_device(&mux_chip->dev);
> -			return ERR_PTR(-EINVAL);
> +			ret = -EINVAL;
> +			goto end;
>  		}
>  
>  		if (args.nargs)
> @@ -634,10 +638,17 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
>  	if (controller >= mux_chip->controllers) {
>  		dev_err(dev, "%pfw: bad mux controller %u specified in %pfw\n",
>  			fwnode, controller, args.fwnode);
> -		put_device(&mux_chip->dev);
> -		return ERR_PTR(-EINVAL);
> +		ret = -EINVAL;
> +		goto end;
>  	}
>  
> +end:
> +	fwnode_handle_put(args.fwnode);
> +	if (ret < 0) {
> +		if (mux_chip)
> +			put_device(&mux_chip->dev);
> +		return ERR_PTR(ret);
> +	}
>  	return &mux_chip->mux[controller];

(nit) A newline between the above } and return would be nice.

>  }
>  
> 
> -- 
> 2.55.0
> 
>