[PATCH] mux: fix use-after-free and node leak in mux_get()

[PATCH] mux: fix use-after-free and node leak in mux_get()

[Wentao Liang]

In mux_get(), after obtaining args.np via of_parse_phandle_with_args(),
of_node_put(args.np) was called immediately. However, args.np is still
used in error paths for dev_err() printing. This leads to a use-after-
free when those error branches are taken.

Fix both issues by moving of_node_put(args.np) after all usage of
args.np, and introducing a common error label "put_and_ret_err" to
centralize node release. Also add missing of_node_put() in all error
path.

Fixes: 84564481bc45 ("mux: Add support for reading mux state from consumer DT node")

Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/mux/core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index a3840fe0995f..517ad3075196 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -564,9 +564,10 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 	}
 
 	mux_chip = of_find_mux_chip_by_node(args.np);
-	of_node_put(args.np);
-	if (!mux_chip)
-		return ERR_PTR(-EPROBE_DEFER);
+	if (!mux_chip) {
+		ret = -EPROBE_DEFER;
+		goto put_and_ret_err;
+	}
 
 	controller = 0;
 	if (state) {
@@ -575,7 +576,8 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 			dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
 				np, args.np);
 			put_device(&mux_chip->dev);
-			return ERR_PTR(-EINVAL);
+			ret = -EINVAL;
+			goto put_and_ret_err;
 		}
 
 		if (args.args_count == 2) {
@@ -591,7 +593,8 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 			dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
 				np, args.np);
 			put_device(&mux_chip->dev);
-			return ERR_PTR(-EINVAL);
+			ret = -EINVAL;
+			goto put_and_ret_err;
 		}
 
 		if (args.args_count)
@@ -602,10 +605,16 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
 		dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
 			np, controller, args.np);
 		put_device(&mux_chip->dev);
-		return ERR_PTR(-EINVAL);
+		ret = -EINVAL;
+		goto put_and_ret_err;
 	}
 
+	of_node_put(args.np);
 	return &mux_chip->mux[controller];
+
+put_and_ret_err:
+	of_node_put(args.np);
+	return ERR_PTR(ret);
 }
 
 /**
-- 
2.34.1

Re: [PATCH] mux: fix use-after-free and node leak in mux_get()

[Markus Elfring]

…
> Fix both issues by moving of_node_put(args.np) after all usage of
> args.np, and introducing a common error label "put_and_ret_err" to
> centralize node release.

I imagine that another label can become helpful also for the statement
“ret = -EINVAL;”.


>                          Also add missing of_node_put() in all error
> path.

You propose to use such a clean-up action for more cases in the implementation
of the function “mux_get”, don't you?
https://elixir.bootlin.com/linux/v7.0-rc7/source/drivers/mux/core.c#L520-L609

How do you think about to increase the application of scope-based resource management?

Regards,
Markus