* [PATCH] clk/ti/adpll: allocate room for terminating null
@ 2019-09-27 14:57 Stephen Kitt
2019-09-27 15:23 ` Tony Lindgren
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Kitt @ 2019-09-27 14:57 UTC (permalink / raw)
To: Tero Kristo, Michael Turquette, Stephen Boyd, linux-omap, linux-clk
Cc: linux-kernel, Stephen Kitt
The buffer allocated in ti_adpll_clk_get_name doesn't account for the
terminating null. This patch adds the extra byte, and switches to
snprintf to avoid overflowing.
Signed-off-by: Stephen Kitt <steve@sk2.org>
---
drivers/clk/ti/adpll.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
index fdfb90058504..27933c4e8a27 100644
--- a/drivers/clk/ti/adpll.c
+++ b/drivers/clk/ti/adpll.c
@@ -196,12 +196,13 @@ static const char *ti_adpll_clk_get_name(struct ti_adpll_data *d,
} else {
const char *base_name = "adpll";
char *buf;
+ size_t size = 8 + 1 + strlen(base_name) + 1 +
+ strlen(postfix) + 1;
- buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
- strlen(postfix), GFP_KERNEL);
+ buf = devm_kzalloc(d->dev, size, GFP_KERNEL);
if (!buf)
return NULL;
- sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
+ snprintf(buf, size, "%08lx.%s.%s", d->pa, base_name, postfix);
name = buf;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] clk/ti/adpll: allocate room for terminating null
2019-09-27 14:57 [PATCH] clk/ti/adpll: allocate room for terminating null Stephen Kitt
@ 2019-09-27 15:23 ` Tony Lindgren
2019-09-27 18:00 ` Stephen Kitt
0 siblings, 1 reply; 12+ messages in thread
From: Tony Lindgren @ 2019-09-27 15:23 UTC (permalink / raw)
To: Stephen Kitt
Cc: Tero Kristo, Michael Turquette, Stephen Boyd, linux-omap,
linux-clk, linux-kernel
* Stephen Kitt <steve@sk2.org> [190927 15:13]:
> The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> terminating null. This patch adds the extra byte, and switches to
> snprintf to avoid overflowing.
>
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
> drivers/clk/ti/adpll.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
> index fdfb90058504..27933c4e8a27 100644
> --- a/drivers/clk/ti/adpll.c
> +++ b/drivers/clk/ti/adpll.c
> @@ -196,12 +196,13 @@ static const char *ti_adpll_clk_get_name(struct ti_adpll_data *d,
> } else {
> const char *base_name = "adpll";
> char *buf;
> + size_t size = 8 + 1 + strlen(base_name) + 1 +
> + strlen(postfix) + 1;
>
> - buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
> - strlen(postfix), GFP_KERNEL);
> + buf = devm_kzalloc(d->dev, size, GFP_KERNEL);
> if (!buf)
> return NULL;
> - sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
> + snprintf(buf, size, "%08lx.%s.%s", d->pa, base_name, postfix);
> name = buf;
> }
>
Thanks for catching this. Maybe just use devm_kasprintf() here?
Regards,
Tony
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] clk/ti/adpll: allocate room for terminating null
2019-09-27 15:23 ` Tony Lindgren
@ 2019-09-27 18:00 ` Stephen Kitt
2019-09-27 18:05 ` [PATCH v2] " Stephen Kitt
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Kitt @ 2019-09-27 18:00 UTC (permalink / raw)
To: Tony Lindgren
Cc: Tero Kristo, Michael Turquette, Stephen Boyd, linux-omap,
linux-clk, linux-kernel
Le 27/09/2019 17:23, Tony Lindgren a écrit :
> * Stephen Kitt <steve@sk2.org> [190927 15:13]:
>> The buffer allocated in ti_adpll_clk_get_name doesn't account for the
>> terminating null. This patch adds the extra byte, and switches to
>> snprintf to avoid overflowing.
>>
>> Signed-off-by: Stephen Kitt <steve@sk2.org>
>> ---
>> drivers/clk/ti/adpll.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
>> index fdfb90058504..27933c4e8a27 100644
>> --- a/drivers/clk/ti/adpll.c
>> +++ b/drivers/clk/ti/adpll.c
>> @@ -196,12 +196,13 @@ static const char *ti_adpll_clk_get_name(struct
>> ti_adpll_data *d,
>> } else {
>> const char *base_name = "adpll";
>> char *buf;
>> + size_t size = 8 + 1 + strlen(base_name) + 1 +
>> + strlen(postfix) + 1;
>>
>> - buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
>> - strlen(postfix), GFP_KERNEL);
>> + buf = devm_kzalloc(d->dev, size, GFP_KERNEL);
>> if (!buf)
>> return NULL;
>> - sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
>> + snprintf(buf, size, "%08lx.%s.%s", d->pa, base_name, postfix);
>> name = buf;
>> }
>>
>
> Thanks for catching this. Maybe just use devm_kasprintf() here?
Ah yes, that would be much better! V2 coming up, thanks for the
suggestion.
Regards,
Stephen
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2] clk/ti/adpll: allocate room for terminating null
2019-09-27 18:00 ` Stephen Kitt
@ 2019-09-27 18:05 ` Stephen Kitt
2019-09-27 18:18 ` Stephen Kitt
2019-10-17 15:48 ` Stephen Boyd
0 siblings, 2 replies; 12+ messages in thread
From: Stephen Kitt @ 2019-09-27 18:05 UTC (permalink / raw)
To: Tero Kristo, Michael Turquette, Stephen Boyd, linux-omap,
linux-clk, Tony Lindgren
Cc: linux-kernel, Stephen Kitt
The buffer allocated in ti_adpll_clk_get_name doesn't account for the
terminating null. This patch switches to ka_sprintf to avoid
overflowing.
Signed-off-by: Stephen Kitt <steve@sk2.org>
---
drivers/clk/ti/adpll.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
index fdfb90058504..021cf9e2b4db 100644
--- a/drivers/clk/ti/adpll.c
+++ b/drivers/clk/ti/adpll.c
@@ -195,14 +195,8 @@ static const char *ti_adpll_clk_get_name(struct ti_adpll_data *d,
return NULL;
} else {
const char *base_name = "adpll";
- char *buf;
-
- buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
- strlen(postfix), GFP_KERNEL);
- if (!buf)
- return NULL;
- sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
- name = buf;
+ name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.%s.%s",
+ d->pa, base_name, postfix);
}
return name;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2] clk/ti/adpll: allocate room for terminating null
2019-09-27 18:05 ` [PATCH v2] " Stephen Kitt
@ 2019-09-27 18:18 ` Stephen Kitt
2019-10-17 15:48 ` Stephen Boyd
1 sibling, 0 replies; 12+ messages in thread
From: Stephen Kitt @ 2019-09-27 18:18 UTC (permalink / raw)
To: Tero Kristo, Michael Turquette, Stephen Boyd, linux-omap,
linux-clk, Tony Lindgren
Cc: linux-kernel
Le 27/09/2019 20:05, Stephen Kitt a écrit :
> The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> terminating null. This patch switches to ka_sprintf to avoid
Aargh, devm_kasprintf of course...
> overflowing.
>
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
> drivers/clk/ti/adpll.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
> index fdfb90058504..021cf9e2b4db 100644
> --- a/drivers/clk/ti/adpll.c
> +++ b/drivers/clk/ti/adpll.c
> @@ -195,14 +195,8 @@ static const char *ti_adpll_clk_get_name(struct
> ti_adpll_data *d,
> return NULL;
> } else {
> const char *base_name = "adpll";
> - char *buf;
> -
> - buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
> - strlen(postfix), GFP_KERNEL);
> - if (!buf)
> - return NULL;
> - sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
> - name = buf;
> + name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.%s.%s",
> + d->pa, base_name, postfix);
> }
>
> return name;
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2] clk/ti/adpll: allocate room for terminating null
2019-09-27 18:05 ` [PATCH v2] " Stephen Kitt
2019-09-27 18:18 ` Stephen Kitt
@ 2019-10-17 15:48 ` Stephen Boyd
2019-10-19 13:54 ` Stephen Kitt
1 sibling, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2019-10-17 15:48 UTC (permalink / raw)
To: Michael Turquette, Stephen Kitt, Tero Kristo, Tony Lindgren,
linux-clk, linux-omap
Cc: linux-kernel, Stephen Kitt
Quoting Stephen Kitt (2019-09-27 11:05:59)
> The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> terminating null. This patch switches to ka_sprintf to avoid
> overflowing.
>
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
> drivers/clk/ti/adpll.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
> index fdfb90058504..021cf9e2b4db 100644
> --- a/drivers/clk/ti/adpll.c
> +++ b/drivers/clk/ti/adpll.c
> @@ -195,14 +195,8 @@ static const char *ti_adpll_clk_get_name(struct ti_adpll_data *d,
> return NULL;
> } else {
> const char *base_name = "adpll";
This is used once.
> - char *buf;
> -
> - buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
> - strlen(postfix), GFP_KERNEL);
> - if (!buf)
> - return NULL;
> - sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
> - name = buf;
> + name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.%s.%s",
So why not make this "%08lx.adpll.%s"?
> + d->pa, base_name, postfix);
> }
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2] clk/ti/adpll: allocate room for terminating null
2019-10-17 15:48 ` Stephen Boyd
@ 2019-10-19 13:54 ` Stephen Kitt
2019-10-19 14:06 ` [PATCH v3] " Stephen Kitt
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Kitt @ 2019-10-19 13:54 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Tero Kristo, Tony Lindgren, linux-clk,
linux-omap, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]
On Thu, 17 Oct 2019 08:48:53 -0700, Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Stephen Kitt (2019-09-27 11:05:59)
> > The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> > terminating null. This patch switches to ka_sprintf to avoid
> > overflowing.
> >
> > Signed-off-by: Stephen Kitt <steve@sk2.org>
> > ---
> > drivers/clk/ti/adpll.c | 10 ++--------
> > 1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
> > index fdfb90058504..021cf9e2b4db 100644
> > --- a/drivers/clk/ti/adpll.c
> > +++ b/drivers/clk/ti/adpll.c
> > @@ -195,14 +195,8 @@ static const char *ti_adpll_clk_get_name(struct
> > ti_adpll_data *d, return NULL;
> > } else {
> > const char *base_name = "adpll";
>
> This is used once.
>
> > - char *buf;
> > -
> > - buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
> > - strlen(postfix), GFP_KERNEL);
> > - if (!buf)
> > - return NULL;
> > - sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
> > - name = buf;
> > + name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.%s.%s",
>
> So why not make this "%08lx.adpll.%s"?
Thanks for the review! I hesitated to do this because I thought the purely
formatting string "%08lx.%s.%s" made the resulting code easier to understand
than a combined "%08lx.adpll.%s". I’ll follow up with a v3 which merges the
"adpll" string into the format string.
Regards,
Stephen
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v3] clk/ti/adpll: allocate room for terminating null
2019-10-19 13:54 ` Stephen Kitt
@ 2019-10-19 14:06 ` Stephen Kitt
2019-10-21 14:31 ` Tony Lindgren
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Stephen Kitt @ 2019-10-19 14:06 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Tero Kristo, Tony Lindgren, linux-clk,
linux-omap, linux-kernel, Stephen Kitt
The buffer allocated in ti_adpll_clk_get_name doesn't account for the
terminating null. This patch switches to devm_kasprintf to avoid
overflowing.
Signed-off-by: Stephen Kitt <steve@sk2.org>
---
Changes since v2:
- Move "adpll" into the format string and drop base_name entirely.
Changes since v1:
- Use devm_kasprintf instead of manually allocating the target
buffer.
---
drivers/clk/ti/adpll.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
index fdfb90058504..bb2f2836dab2 100644
--- a/drivers/clk/ti/adpll.c
+++ b/drivers/clk/ti/adpll.c
@@ -194,15 +194,8 @@ static const char *ti_adpll_clk_get_name(struct ti_adpll_data *d,
if (err)
return NULL;
} else {
- const char *base_name = "adpll";
- char *buf;
-
- buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
- strlen(postfix), GFP_KERNEL);
- if (!buf)
- return NULL;
- sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
- name = buf;
+ name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.adpll.%s",
+ d->pa, postfix);
}
return name;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3] clk/ti/adpll: allocate room for terminating null
2019-10-19 14:06 ` [PATCH v3] " Stephen Kitt
@ 2019-10-21 14:31 ` Tony Lindgren
2019-11-08 17:00 ` Stephen Boyd
2019-11-08 17:01 ` Stephen Boyd
2 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2019-10-21 14:31 UTC (permalink / raw)
To: Stephen Kitt
Cc: Stephen Boyd, Michael Turquette, Tero Kristo, linux-clk,
linux-omap, linux-kernel
* Stephen Kitt <steve@sk2.org> [191019 14:07]:
> The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> terminating null. This patch switches to devm_kasprintf to avoid
> overflowing.
>
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
> Changes since v2:
> - Move "adpll" into the format string and drop base_name entirely.
>
> Changes since v1:
> - Use devm_kasprintf instead of manually allocating the target
> buffer.
Acked-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/clk/ti/adpll.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
> index fdfb90058504..bb2f2836dab2 100644
> --- a/drivers/clk/ti/adpll.c
> +++ b/drivers/clk/ti/adpll.c
> @@ -194,15 +194,8 @@ static const char *ti_adpll_clk_get_name(struct ti_adpll_data *d,
> if (err)
> return NULL;
> } else {
> - const char *base_name = "adpll";
> - char *buf;
> -
> - buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
> - strlen(postfix), GFP_KERNEL);
> - if (!buf)
> - return NULL;
> - sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
> - name = buf;
> + name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.adpll.%s",
> + d->pa, postfix);
> }
>
> return name;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3] clk/ti/adpll: allocate room for terminating null
2019-10-19 14:06 ` [PATCH v3] " Stephen Kitt
2019-10-21 14:31 ` Tony Lindgren
@ 2019-11-08 17:00 ` Stephen Boyd
2019-11-08 20:17 ` Stephen Kitt
2019-11-08 17:01 ` Stephen Boyd
2 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2019-11-08 17:00 UTC (permalink / raw)
To: Stephen Kitt
Cc: Michael Turquette, Tero Kristo, Tony Lindgren, linux-clk,
linux-omap, linux-kernel, Stephen Kitt
Quoting Stephen Kitt (2019-10-19 07:06:34)
> The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> terminating null. This patch switches to devm_kasprintf to avoid
> overflowing.
>
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
Please don't send as replies to existing threads. It screws up my
tooling and makes it more manual to apply the patch. I guess I'll have
to go fix my scripts to ignore certain emails.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] clk/ti/adpll: allocate room for terminating null
2019-11-08 17:00 ` Stephen Boyd
@ 2019-11-08 20:17 ` Stephen Kitt
0 siblings, 0 replies; 12+ messages in thread
From: Stephen Kitt @ 2019-11-08 20:17 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Tero Kristo, Tony Lindgren, linux-clk,
linux-omap, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 662 bytes --]
On Fri, 08 Nov 2019 09:00:25 -0800, Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Stephen Kitt (2019-10-19 07:06:34)
> > The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> > terminating null. This patch switches to devm_kasprintf to avoid
> > overflowing.
> >
> > Signed-off-by: Stephen Kitt <steve@sk2.org>
> > ---
>
> Please don't send as replies to existing threads. It screws up my
> tooling and makes it more manual to apply the patch. I guess I'll have
> to go fix my scripts to ignore certain emails.
My bad, sorry about that, I misread the In-Reply-To section of
submitting-patches :-(.
Regards,
Stephen
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] clk/ti/adpll: allocate room for terminating null
2019-10-19 14:06 ` [PATCH v3] " Stephen Kitt
2019-10-21 14:31 ` Tony Lindgren
2019-11-08 17:00 ` Stephen Boyd
@ 2019-11-08 17:01 ` Stephen Boyd
2 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2019-11-08 17:01 UTC (permalink / raw)
To: Stephen Kitt
Cc: Michael Turquette, Tero Kristo, Tony Lindgren, linux-clk,
linux-omap, linux-kernel, Stephen Kitt
Quoting Stephen Kitt (2019-10-19 07:06:34)
> The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> terminating null. This patch switches to devm_kasprintf to avoid
> overflowing.
>
> Signed-off-by: Stephen Kitt <steve@sk2.org>
> ---
Applied to clk-next
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-11-08 21:41 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-27 14:57 [PATCH] clk/ti/adpll: allocate room for terminating null Stephen Kitt
2019-09-27 15:23 ` Tony Lindgren
2019-09-27 18:00 ` Stephen Kitt
2019-09-27 18:05 ` [PATCH v2] " Stephen Kitt
2019-09-27 18:18 ` Stephen Kitt
2019-10-17 15:48 ` Stephen Boyd
2019-10-19 13:54 ` Stephen Kitt
2019-10-19 14:06 ` [PATCH v3] " Stephen Kitt
2019-10-21 14:31 ` Tony Lindgren
2019-11-08 17:00 ` Stephen Boyd
2019-11-08 20:17 ` Stephen Kitt
2019-11-08 17:01 ` Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).