On Sat, Nov 19, 2022 at 09:05:28AM +0100, Christophe JAILLET wrote: > Le 19/11/2022 à 00:31, Kees Cook a écrit : > > With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG), > > indirect call targets are validated against the expected function > > pointer prototype to make sure the call target is valid to help mitigate > > ROP attacks. If they are not identical, there is a failure at run time, > > which manifests as either a kernel panic or thread getting killed. > > > > msc313_rtc_probe() was passing clk_disable_unprepare() directly, which > > did not have matching prototypes for devm_add_action_or_reset()'s callback > > argument. Add a wrapper and remove the cast. > > > > This was found as a result of Clang's new -Wcast-function-type-strict > > flag, which is more sensitive than the simpler -Wcast-function-type, > > which only checks for type width mismatches. > > > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > Link: https://lore.kernel.org/lkml/202211041527.HD8TLSE1-lkp@xxxxxxxxx > > Cc: Daniel Palmer <daniel@xxxxxxxxx> > > Cc: Romain Perier <romain.perier@xxxxxxxxx> > > Cc: Alessandro Zummo <a.zummo@xxxxxxxxxxxx> > > Cc: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> > > Cc: "Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx> > > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > > Cc: linux-rtc@xxxxxxxxxxxxxxx > > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > > --- > > drivers/rtc/rtc-msc313.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/rtc/rtc-msc313.c b/drivers/rtc/rtc-msc313.c > > index f3fde013c4b8..36e3e77f303e 100644 > > --- a/drivers/rtc/rtc-msc313.c > > +++ b/drivers/rtc/rtc-msc313.c > > @@ -177,6 +177,13 @@ static irqreturn_t msc313_rtc_interrupt(s32 irq, void *dev_id) > > return IRQ_HANDLED; > > } > > +static void msc313_clk_disable_unprepare(void *data) > > +{ > > + struct clk *clk = data; > > + > > + clk_disable_unprepare(clk); > > +} > > + > > static int msc313_rtc_probe(struct platform_device *pdev) > > { > > struct device *dev = &pdev->dev; > > @@ -224,7 +231,7 @@ static int msc313_rtc_probe(struct platform_device *pdev) > > return ret; > > } > > - ret = devm_add_action_or_reset(dev, (void (*) (void *))clk_disable_unprepare, clk); > > + ret = devm_add_action_or_reset(dev, msc313_clk_disable_unprepare, clk); > > if (ret) > > return ret; > > Hi, > > another way to fix it, is to use devm_clk_get_enabled(). > > It removes some LoC instead of introducing some new ones and saves a few > bytes of memory. Hrm, I'm not familiar with the clk stuff here -- how do I use it? Should it just be like this? (The NULL argument is ok?) diff --git a/drivers/rtc/rtc-msc313.c b/drivers/rtc/rtc-msc313.c index f3fde013c4b8..8d7737e0e2e0 100644 --- a/drivers/rtc/rtc-msc313.c +++ b/drivers/rtc/rtc-msc313.c @@ -212,22 +212,12 @@ static int msc313_rtc_probe(struct platform_device *pdev) return ret; } - clk = devm_clk_get(dev, NULL); + clk = devm_clk_get_enabled(dev, NULL); if (IS_ERR(clk)) { dev_err(dev, "No input reference clock\n"); return PTR_ERR(clk); } - ret = clk_prepare_enable(clk); - if (ret) { - dev_err(dev, "Failed to enable the reference clock, %d\n", ret); - return ret; - } - - ret = devm_add_action_or_reset(dev, (void (*) (void *))clk_disable_unprepare, clk); - if (ret) - return ret; - rate = clk_get_rate(clk); writew(rate & 0xFFFF, priv->rtc_base + REG_RTC_FREQ_CW_L); writew((rate >> 16) & 0xFFFF, priv->rtc_base + REG_RTC_FREQ_CW_H); -- Kees Cook
