| Message ID | E1rl62V-004UFh-Te@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clkdev: report over-sized strings when creating clkdev entries | expand |
Quoting Russell King (Oracle) (2024-03-15 04:47:55) > Report an error when an attempt to register a clkdev entry results in a > truncated string so the problem can be easily spotted. > > Reported by: Duanqiang Wen <duanqiangwen@net-swift.com> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- Russell, are you taking this through your tree? I took the last one because it was small and you reviewed it instead of applied it. If you're taking it please add: Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Quoting Russell King (Oracle) (2024-03-15 04:47:55) > Report an error when an attempt to register a clkdev entry results in a > truncated string so the problem can be easily spotted. > > Reported by: Duanqiang Wen <duanqiangwen@net-swift.com> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- Applied to clk-next
Quoting Stephen Boyd (2024-05-01 17:59:16) > Quoting Russell King (Oracle) (2024-03-15 04:47:55) > > Report an error when an attempt to register a clkdev entry results in a > > truncated string so the problem can be easily spotted. > > > > Reported by: Duanqiang Wen <duanqiangwen@net-swift.com> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > --- > > Applied to clk-next > And backed out because I get a compilation failure drivers/clk/clkdev.c: In function 'vclkdev_alloc': drivers/clk/clkdev.c:182:17: error: function 'vclkdev_alloc' might be a candidate for 'gnu_printf' format attribute [-Werror=suggest-attribute=format] 182 | res = vsnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap); | ^~~ cc1: all warnings being treated as errors make[5]: *** [scripts/Makefile.build:244: drivers/clk/clkdev.o] Error 1 make[4]: *** [scripts/Makefile.build:485: drivers/clk] Error 2
On Wed, May 01, 2024 at 06:02:54PM -0700, Stephen Boyd wrote: > Quoting Stephen Boyd (2024-05-01 17:59:16) > > Quoting Russell King (Oracle) (2024-03-15 04:47:55) > > > Report an error when an attempt to register a clkdev entry results in a > > > truncated string so the problem can be easily spotted. > > > > > > Reported by: Duanqiang Wen <duanqiangwen@net-swift.com> > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > --- > > > > Applied to clk-next > > > > And backed out because I get a compilation failure > > drivers/clk/clkdev.c: In function 'vclkdev_alloc': > drivers/clk/clkdev.c:182:17: error: function 'vclkdev_alloc' might be a candidate for 'gnu_printf' format attribute [-Werror=suggest-attribute=format] > 182 | res = vsnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap); > | ^~~ > cc1: all warnings being treated as errors > make[5]: *** [scripts/Makefile.build:244: drivers/clk/clkdev.o] Error 1 > make[4]: *** [scripts/Makefile.build:485: drivers/clk] Error 2 It builds fine for me. I don't get this _error_, and it's really no different from what it originally was - instead of using vcsnprintf() we're now using vsnprintf(). That should make no difference what so ever.
On Thu, May 02, 2024 at 09:02:16AM +0100, Russell King (Oracle) wrote: > On Wed, May 01, 2024 at 06:02:54PM -0700, Stephen Boyd wrote: > > Quoting Stephen Boyd (2024-05-01 17:59:16) > > > Quoting Russell King (Oracle) (2024-03-15 04:47:55) > > > > Report an error when an attempt to register a clkdev entry results in a > > > > truncated string so the problem can be easily spotted. > > > > > > > > Reported by: Duanqiang Wen <duanqiangwen@net-swift.com> > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > --- > > > > > > Applied to clk-next > > > > > > > And backed out because I get a compilation failure > > > > drivers/clk/clkdev.c: In function 'vclkdev_alloc': > > drivers/clk/clkdev.c:182:17: error: function 'vclkdev_alloc' might be a candidate for 'gnu_printf' format attribute [-Werror=suggest-attribute=format] > > 182 | res = vsnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap); > > | ^~~ > > cc1: all warnings being treated as errors > > make[5]: *** [scripts/Makefile.build:244: drivers/clk/clkdev.o] Error 1 > > make[4]: *** [scripts/Makefile.build:485: drivers/clk] Error 2 > > It builds fine for me. I don't get this _error_, and it's really no > different from what it originally was - instead of using vcsnprintf() > we're now using vsnprintf(). That should make no difference what so > ever. ... and I've just checked, and it builds entirely cleanly for me. I'll merge it.
On Thu, May 02, 2024 at 09:22:39AM +0100, Russell King (Oracle) wrote: > On Thu, May 02, 2024 at 09:02:16AM +0100, Russell King (Oracle) wrote: > > On Wed, May 01, 2024 at 06:02:54PM -0700, Stephen Boyd wrote: > > > Quoting Stephen Boyd (2024-05-01 17:59:16) > > > > Quoting Russell King (Oracle) (2024-03-15 04:47:55) > > > > > Report an error when an attempt to register a clkdev entry results in a > > > > > truncated string so the problem can be easily spotted. > > > > > > > > > > Reported by: Duanqiang Wen <duanqiangwen@net-swift.com> > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > > --- > > > > > > > > Applied to clk-next > > > > > > > > > > And backed out because I get a compilation failure > > > > > > drivers/clk/clkdev.c: In function 'vclkdev_alloc': > > > drivers/clk/clkdev.c:182:17: error: function 'vclkdev_alloc' might be a candidate for 'gnu_printf' format attribute [-Werror=suggest-attribute=format] > > > 182 | res = vsnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap); > > > | ^~~ > > > cc1: all warnings being treated as errors > > > make[5]: *** [scripts/Makefile.build:244: drivers/clk/clkdev.o] Error 1 > > > make[4]: *** [scripts/Makefile.build:485: drivers/clk] Error 2 > > > > It builds fine for me. I don't get this _error_, and it's really no > > different from what it originally was - instead of using vcsnprintf() > > we're now using vsnprintf(). That should make no difference what so > > ever. > > ... and I've just checked, and it builds entirely cleanly for me. > > I'll merge it. It should be in tonight's linux-next tree. I also note... the kernel build bot never flagged the above, not even a warning for it, and it would've built the patch as it was Cc'd to linux-kernel. So my conclusion is the error you are seeing is somehow specific to your environment. Maybe you're building with an additional set of warning flags and -Werror?
Quoting Russell King (Oracle) (2024-05-02 04:08:38) > On Thu, May 02, 2024 at 09:22:39AM +0100, Russell King (Oracle) wrote: > > On Thu, May 02, 2024 at 09:02:16AM +0100, Russell King (Oracle) wrote: > > > On Wed, May 01, 2024 at 06:02:54PM -0700, Stephen Boyd wrote: > > > > Quoting Stephen Boyd (2024-05-01 17:59:16) > > > > > Quoting Russell King (Oracle) (2024-03-15 04:47:55) > > > > > > Report an error when an attempt to register a clkdev entry results in a > > > > > > truncated string so the problem can be easily spotted. > > > > > > > > > > > > Reported by: Duanqiang Wen <duanqiangwen@net-swift.com> > > > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > > > > > --- > > > > > > > > > > Applied to clk-next > > > > > > > > > > > > > And backed out because I get a compilation failure > > > > > > > > drivers/clk/clkdev.c: In function 'vclkdev_alloc': > > > > drivers/clk/clkdev.c:182:17: error: function 'vclkdev_alloc' might be a candidate for 'gnu_printf' format attribute [-Werror=suggest-attribute=format] > > > > 182 | res = vsnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap); > > > > | ^~~ > > > > cc1: all warnings being treated as errors > > > > make[5]: *** [scripts/Makefile.build:244: drivers/clk/clkdev.o] Error 1 > > > > make[4]: *** [scripts/Makefile.build:485: drivers/clk] Error 2 > > > > > > It builds fine for me. I don't get this _error_, and it's really no > > > different from what it originally was - instead of using vcsnprintf() > > > we're now using vsnprintf(). That should make no difference what so > > > ever. > > > > ... and I've just checked, and it builds entirely cleanly for me. > > > > I'll merge it. > > It should be in tonight's linux-next tree. Ok thanks > > I also note... the kernel build bot never flagged the above, not even > a warning for it, and it would've built the patch as it was Cc'd to > linux-kernel. Indeed. I see a report here[1] but it's only a warning. And there was some work a few years ago[2] that I forgot about. Seems there was a possible fix[3] as well. > So my conclusion is the error you are seeing is somehow > specific to your environment. Maybe you're building with an additional > set of warning flags and -Werror? > Yes I build with W=1 but I didn't think that turned warnings into errors. Maybe that part is new. [1] https://lore.kernel.org/all/202403110643.27JXEVCI-lkp@intel.com/ [2] https://lore.kernel.org/all/20210310085937.GF4931@dell/ [3] https://lore.kernel.org/all/20221111071809.3440-1-liubo03@inspur.com/
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index ee37d0be6877..3146f26ab997 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -158,6 +158,10 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, va_list ap) { struct clk_lookup_alloc *cla; + struct va_format fmt; + const char *failure; + size_t max_size; + ssize_t res; cla = kzalloc(sizeof(*cla), GFP_KERNEL); if (!cla) @@ -165,16 +169,34 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, cla->cl.clk_hw = hw; if (con_id) { - strscpy(cla->con_id, con_id, sizeof(cla->con_id)); + res = strscpy(cla->con_id, con_id, sizeof(cla->con_id)); + if (res < 0) { + max_size = sizeof(cla->con_id); + failure = "connection"; + goto fail; + } cla->cl.con_id = cla->con_id; } if (dev_fmt) { - vscnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap); + res = vsnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap); + if (res >= sizeof(cla->dev_id)) { + max_size = sizeof(cla->dev_id); + failure = "device"; + goto fail; + } cla->cl.dev_id = cla->dev_id; } return &cla->cl; + +fail: + fmt.fmt = dev_fmt; + fmt.va = ≈ + pr_err("%pV:%s: %s ID is greater than %zu\n", + &fmt, con_id, failure, max_size); + kfree(cla); + return NULL; } static struct clk_lookup *
Report an error when an attempt to register a clkdev entry results in a truncated string so the problem can be easily spotted. Reported by: Duanqiang Wen <duanqiangwen@net-swift.com> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/clk/clkdev.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)