Messages in this thread | | | Date | Fri, 28 Jan 2022 11:20:11 +0100 | From | Greg KH <> | Subject | Re: [PATCH v2] clk: x86: Fix a wild pointer dereference in fch_clk_probe() |
| |
On Fri, Jan 07, 2022 at 03:15:58PM +0800, Zhou Qingyang wrote: > In fch_clk_probe(), the return value of clk_hw_register_mux() is > assigned to hws[ST_CLK_MUX] and there is a dereference of it in > fch_clk_probe(), which could lead to a wild pointer dereference on > failure of clk_hw_register_mux(). > > Fix this bug by adding a check of hws[ST_CLK_MUX]. > > This bug was found by a static analyzer. > > Builds with CONFIG_X86_AMD_PLATFORM_DEVICE=y show no new warnings, and > our static analyzer no longer warns about this code. > > Fixes: 19fe87fd854a ("clk: x86: Support RV architecture") > Signed-off-by: Zhou Qingyang <zhou1615@umn.edu> > --- > The analysis employs differential checking to identify inconsistent > security operations (e.g., checks or kfrees) between two code paths > and confirms that the inconsistent operations are not recovered in > the current function or the callers, so they constitute bugs. > > Note that, as a bug found by static analysis, it can be a false > positive or hard to trigger. Multiple researchers have cross-reviewed > the bug. > > Changes in v2: > - Add error check to every register function calls. > - Add error handling logic to every error path. > - Turn clk_hw_register_mux to devm_clk_hw_register_mux. > - Add error check of clk_set_parent(). > > drivers/clk/x86/clk-fch.c | 53 ++++++++++++++++++++++++++++++++++++--- > 1 file changed, 49 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/x86/clk-fch.c b/drivers/clk/x86/clk-fch.c > index 8f7c5142b0f0..47754761b19c 100644 > --- a/drivers/clk/x86/clk-fch.c > +++ b/drivers/clk/x86/clk-fch.c > @@ -36,6 +36,7 @@ static struct clk_hw *hws[ST_MAX_CLKS]; > static int fch_clk_probe(struct platform_device *pdev) > { > struct fch_clk_data *fch_data; > + int ret; > > fch_data = dev_get_platdata(&pdev->dev); > if (!fch_data || !fch_data->base) > @@ -44,35 +45,79 @@ static int fch_clk_probe(struct platform_device *pdev) > if (!fch_data->is_rv) { > hws[ST_CLK_48M] = clk_hw_register_fixed_rate(NULL, "clk48MHz", > NULL, 0, 48000000); > + if (IS_ERR(hws[ST_CLK_48M])) > + return PTR_ERR(hws[ST_CLK_48M]); > + > hws[ST_CLK_25M] = clk_hw_register_fixed_rate(NULL, "clk25MHz", > NULL, 0, 25000000); > + if (IS_ERR(hws[ST_CLK_25M])) { > + ret = PTR_ERR(hws[ST_CLK_25M]); > + goto err_st_clk_25m; > + } > > - hws[ST_CLK_MUX] = clk_hw_register_mux(NULL, "oscout1_mux", > + hws[ST_CLK_MUX] = devm_clk_hw_register_mux(NULL, "oscout1_mux", > clk_oscout1_parents, ARRAY_SIZE(clk_oscout1_parents), > 0, fch_data->base + CLKDRVSTR2, OSCOUT1CLK25MHZ, 3, 0, > NULL); > + if (IS_ERR(hws[ST_CLK_MUX])) { > + ret = PTR_ERR(hws[ST_CLK_MUX]); > + goto err_st_clk_mux; > + } > > - clk_set_parent(hws[ST_CLK_MUX]->clk, hws[ST_CLK_48M]->clk); > + ret = clk_set_parent(hws[ST_CLK_MUX]->clk, hws[ST_CLK_48M]->clk); > + if (ret) > + goto err_clk_set_parent; > > hws[ST_CLK_GATE] = clk_hw_register_gate(NULL, "oscout1", > "oscout1_mux", 0, fch_data->base + MISCCLKCNTL1, > OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL); > + if (IS_ERR(hws[ST_CLK_GATE])) { > + ret = PTR_ERR(hws[ST_CLK_GATE]); > + goto err_st_clk_gate; > + } > > - devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], > + ret = devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE], > "oscout1", NULL); > + if (ret) > + goto err_register_st_clk_gate; > } else { > hws[RV_CLK_48M] = clk_hw_register_fixed_rate(NULL, "clk48MHz", > NULL, 0, 48000000); > + if (IS_ERR(hws[RV_CLK_48M])) > + return PTR_ERR(hws[RV_CLK_48M]); > > hws[RV_CLK_GATE] = clk_hw_register_gate(NULL, "oscout1", > "clk48MHz", 0, fch_data->base + MISCCLKCNTL1, > OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL); > + if (IS_ERR(hws[RV_CLK_GATE])) { > + ret = PTR_ERR(hws[RV_CLK_GATE]); > + goto err_rv_clk_gate; > + } > > - devm_clk_hw_register_clkdev(&pdev->dev, hws[RV_CLK_GATE], > + ret = devm_clk_hw_register_clkdev(&pdev->dev, hws[RV_CLK_GATE], > "oscout1", NULL); > + if (ret) > + goto err_register_rv_clk_gate; > } > > return 0; > + > +err_register_st_clk_gate: > + clk_hw_unregister_gate(hws[ST_CLK_GATE]); > +err_st_clk_gate: > +err_clk_set_parent: > + clk_hw_unregister_mux(hws[ST_CLK_MUX]); > +err_st_clk_mux: > + clk_hw_unregister_fixed_rate(hws[ST_CLK_25M]); > +err_st_clk_25m: > + clk_hw_unregister_fixed_rate(hws[ST_CLK_48M]); > + return ret; > + > +err_register_rv_clk_gate: > + clk_hw_unregister_gate(hws[RV_CLK_GATE]); > +err_rv_clk_gate: > + clk_hw_unregister_fixed_rate(hws[RV_CLK_48M]); > + return ret; > } > > static int fch_clk_remove(struct platform_device *pdev) > -- > 2.25.1 >
As stated before, umn.edu is still not allowed to contribute to the Linux kernel. Please work with your administration to resolve this issue.
| |