2024-03-15 11:48:19

by Russell King (Oracle)

[permalink] [raw]
Subject: [PATCH] clkdev: report over-sized strings when creating clkdev entries

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 <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/clk/clkdev.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)

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 = &ap;
+ 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 *
--
2.30.2



2024-04-08 03:48:54

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] clkdev: report over-sized strings when creating clkdev entries

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 <[email protected]>
> Signed-off-by: Russell King (Oracle) <[email protected]>
> ---

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 <[email protected]>

2024-05-02 00:59:26

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] clkdev: report over-sized strings when creating clkdev entries

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 <[email protected]>
> Signed-off-by: Russell King (Oracle) <[email protected]>
> ---

Applied to clk-next

2024-05-02 01:03:11

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] clkdev: report over-sized strings when creating clkdev entries

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 <[email protected]>
> > Signed-off-by: Russell King (Oracle) <[email protected]>
> > ---
>
> 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

2024-05-02 08:02:51

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] clkdev: report over-sized strings when creating clkdev entries

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 <[email protected]>
> > > Signed-off-by: Russell King (Oracle) <[email protected]>
> > > ---
> >
> > 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.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2024-05-02 08:23:04

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] clkdev: report over-sized strings when creating clkdev entries

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 <[email protected]>
> > > > Signed-off-by: Russell King (Oracle) <[email protected]>
> > > > ---
> > >
> > > 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.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2024-05-02 11:11:17

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] clkdev: report over-sized strings when creating clkdev entries

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 <[email protected]>
> > > > > Signed-off-by: Russell King (Oracle) <[email protected]>
> > > > > ---
> > > >
> > > > 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?

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2024-05-02 22:18:12

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] clkdev: report over-sized strings when creating clkdev entries

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 <[email protected]>
> > > > > > Signed-off-by: Russell King (Oracle) <[email protected]>
> > > > > > ---
> > > > >
> > > > > 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/[email protected]/
[2] https://lore.kernel.org/all/20210310085937.GF4931@dell/
[3] https://lore.kernel.org/all/[email protected]/