Messages in this thread | | | Date | Wed, 13 Mar 2024 10:09:04 +0000 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH clk] clk: clkdev: add error messages for name exceeding maximum length |
| |
On Wed, Mar 13, 2024 at 02:42:52PM +0800, Duanqiang Wen wrote: > diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c > index ee37d0be6877..620dc1e80b48 100644 > --- a/drivers/clk/clkdev.c > +++ b/drivers/clk/clkdev.c > @@ -158,6 +158,9 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, > va_list ap) > { > struct clk_lookup_alloc *cla; > + struct device *dev; > + > + dev = clk_hw_get_dev(hw);
Sorry, but no, clkdev should have minimal dependencies on CCF (it was designed to be completely independent). I'd prefer not to add this. Just print the formatted dev_fmt+ap and the con_id when reporting errors.
> > cla = kzalloc(sizeof(*cla), GFP_KERNEL); > if (!cla) > @@ -165,11 +168,19 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, > > cla->cl.clk_hw = hw; > if (con_id) { > + if (strlen(dev_fmt) >= MAX_CON_ID) {
This is wrong (uses dev_fmt not con_id). Also, use sizeof(cla->con_id) to test against.
> + pr_err("%s:con_id string cannot be greater than 16 characters\n", dev_fmt);
Cleanup?
> + return NULL; > + } > strscpy(cla->con_id, con_id, sizeof(cla->con_id)); > cla->cl.con_id = cla->con_id; > } > > if (dev_fmt) { > + if (strlen(dev_fmt) >= MAX_DEV_ID) {
This is also wrong. The length of the format string does not give any information on how long the resulting string actually is.
> + pr_err("%s:dev_id string cannot be greater than 20 characters\n", dev_fmt);
Cleanup?
> + return NULL; > + } > vscnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap);
Using vsnprintf() here and checking whether the return value is larger than sizeof(cla->dev_id) would be better.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
| |