* [PATCH v2 1/3] jbd2: Avoid printing outside the boundary of the buffer
2023-03-23 12:37 [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
@ 2023-03-23 12:37 ` Andy Shevchenko
2023-03-23 12:37 ` [PATCH v2 2/3] lib/string_helpers: Change returned value of the strreplace() Andy Shevchenko
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2023-03-23 12:37 UTC (permalink / raw)
To: Kees Cook, Greg Kroah-Hartman, Andy Shevchenko, Cezary Rojewski,
linux-ext4, linux-kernel
Cc: Theodore Ts'o, Jan Kara, Andy Shevchenko, Rafael J. Wysocki,
Jan Kara
Theoretically possible that "%pg" will take all room for the j_devname
and hence the "-%lu" will go outside the boundary due to unconditional
sprintf() in use. To make this code more robust, replace two sequential
s*printf():s by a single call and then replace forbidden character.
It's possible to do this way, because '/' won't ever be in the result
of "-%lu".
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
fs/jbd2/journal.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 8ae419152ff6..6e17f8f94dfd 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1491,7 +1491,6 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
{
journal_t *journal;
sector_t blocknr;
- char *p;
int err = 0;
blocknr = 0;
@@ -1515,9 +1514,8 @@ journal_t *jbd2_journal_init_inode(struct inode *inode)
journal->j_inode = inode;
snprintf(journal->j_devname, sizeof(journal->j_devname),
- "%pg", journal->j_dev);
- p = strreplace(journal->j_devname, '/', '!');
- sprintf(p, "-%lu", journal->j_inode->i_ino);
+ "%pg-%lu", journal->j_dev, journal->j_inode->i_ino);
+ strreplace(journal->j_devname, '/', '!');
jbd2_stats_proc_init(journal);
return journal;
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 2/3] lib/string_helpers: Change returned value of the strreplace()
2023-03-23 12:37 [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
2023-03-23 12:37 ` [PATCH v2 1/3] jbd2: Avoid printing outside the boundary of the buffer Andy Shevchenko
@ 2023-03-23 12:37 ` Andy Shevchenko
2023-03-23 12:37 ` [PATCH v2 3/3] kobject: Use return value of strreplace() Andy Shevchenko
2023-04-05 13:34 ` [PATCH v2 0/3] lib/string_helpers et al.: Change " Andy Shevchenko
3 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2023-03-23 12:37 UTC (permalink / raw)
To: Kees Cook, Greg Kroah-Hartman, Andy Shevchenko, Cezary Rojewski,
linux-ext4, linux-kernel
Cc: Theodore Ts'o, Jan Kara, Andy Shevchenko, Rafael J. Wysocki
It's more useful to return the pointer to the string itself
with strreplace(), so it may be used like
attr->name = strreplace(name, '/', '_');
While at it, amend the kernel documentation.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/linux/string.h | 2 +-
lib/string_helpers.c | 12 ++++++++----
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index a7bff7ed3cb0..cb0c24ce0826 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -169,7 +169,7 @@ static inline void memcpy_flushcache(void *dst, const void *src, size_t cnt)
#endif
void *memchr_inv(const void *s, int c, size_t n);
-char *strreplace(char *s, char old, char new);
+char *strreplace(char *str, char old, char new);
extern void kfree_const(const void *x);
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 230020a2e076..d3b1dd718daf 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -979,18 +979,22 @@ EXPORT_SYMBOL(__sysfs_match_string);
/**
* strreplace - Replace all occurrences of character in string.
- * @s: The string to operate on.
+ * @str: The string to operate on.
* @old: The character being replaced.
* @new: The character @old is replaced with.
*
- * Returns pointer to the nul byte at the end of @s.
+ * Replaces the each @old character with a @new one in the given string @str.
+ *
+ * Return: pointer to the string @str itself.
*/
-char *strreplace(char *s, char old, char new)
+char *strreplace(char *str, char old, char new)
{
+ char *s = str;
+
for (; *s; ++s)
if (*s == old)
*s = new;
- return s;
+ return str;
}
EXPORT_SYMBOL(strreplace);
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 3/3] kobject: Use return value of strreplace()
2023-03-23 12:37 [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
2023-03-23 12:37 ` [PATCH v2 1/3] jbd2: Avoid printing outside the boundary of the buffer Andy Shevchenko
2023-03-23 12:37 ` [PATCH v2 2/3] lib/string_helpers: Change returned value of the strreplace() Andy Shevchenko
@ 2023-03-23 12:37 ` Andy Shevchenko
2023-03-26 13:37 ` David Laight
2023-04-05 13:34 ` [PATCH v2 0/3] lib/string_helpers et al.: Change " Andy Shevchenko
3 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2023-03-23 12:37 UTC (permalink / raw)
To: Kees Cook, Greg Kroah-Hartman, Andy Shevchenko, Cezary Rojewski,
linux-ext4, linux-kernel
Cc: Theodore Ts'o, Jan Kara, Andy Shevchenko, Rafael J. Wysocki
Since strreplace() returns the pointer to the string itself,
we may use it directly in the code.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
lib/kobject.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lib/kobject.c b/lib/kobject.c
index f79a434e1231..16d530f9c174 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -281,8 +281,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
kfree_const(s);
if (!t)
return -ENOMEM;
- strreplace(t, '/', '!');
- s = t;
+ s = strreplace(t, '/', '!');
}
kfree_const(kobj->name);
kobj->name = s;
--
2.40.0.1.gaa8946217a0b
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH v2 3/3] kobject: Use return value of strreplace()
2023-03-23 12:37 ` [PATCH v2 3/3] kobject: Use return value of strreplace() Andy Shevchenko
@ 2023-03-26 13:37 ` David Laight
0 siblings, 0 replies; 12+ messages in thread
From: David Laight @ 2023-03-26 13:37 UTC (permalink / raw)
To: 'Andy Shevchenko', Kees Cook, Greg Kroah-Hartman,
Cezary Rojewski, linux-ext4@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: Theodore Ts'o, Jan Kara, Andy Shevchenko, Rafael J. Wysocki
From: Andy Shevchenko
> Sent: 23 March 2023 12:37
>
> Since strreplace() returns the pointer to the string itself,
> we may use it directly in the code.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> lib/kobject.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/kobject.c b/lib/kobject.c
> index f79a434e1231..16d530f9c174 100644
> --- a/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -281,8 +281,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
> kfree_const(s);
> if (!t)
> return -ENOMEM;
> - strreplace(t, '/', '!');
> - s = t;
> + s = strreplace(t, '/', '!');
Why do this? It just makes the code harder to read because
you have to know another 'silly fact' about a function.
Possibly useful return values might be:
1) The address of the first changed character.
2) The address of the last changed characher.
3) The '\0' terminator.
4) void.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()
2023-03-23 12:37 [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
` (2 preceding siblings ...)
2023-03-23 12:37 ` [PATCH v2 3/3] kobject: Use return value of strreplace() Andy Shevchenko
@ 2023-04-05 13:34 ` Andy Shevchenko
2023-04-05 14:24 ` Greg Kroah-Hartman
3 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2023-04-05 13:34 UTC (permalink / raw)
To: Kees Cook, Greg Kroah-Hartman, Cezary Rojewski, linux-ext4,
linux-kernel
Cc: Theodore Ts'o, Jan Kara, Rafael J. Wysocki
On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> It's more convenient to have strreplace() to return the pointer to
> the string itself. This will help users to make their code better.
>
> The patch 1 kills the only user of the returned value of strreplace(),
> Patch 2 converts the return value of strreplace(). And patch 3 shows
> how it may be useful. That said, the series can be routed via fs tree,
> with or without the last patch.
Since there are no comments, who can apply this (patches 1 and 2)?
Greg, are you fine with the kobject change?
> In v2:
> - removed not anymore used variable (LKP)
> - added tag (Jan)
> - fixed wording (Kees)
> - actually return the pointer to the string itself
>
> Andy Shevchenko (3):
> jbd2: Avoid printing outside the boundary of the buffer
> lib/string_helpers: Change returned value of the strreplace()
> kobject: Use return value of strreplace()
>
> fs/jbd2/journal.c | 6 ++----
> include/linux/string.h | 2 +-
> lib/kobject.c | 3 +--
> lib/string_helpers.c | 12 ++++++++----
> 4 files changed, 12 insertions(+), 11 deletions(-)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()
2023-04-05 13:34 ` [PATCH v2 0/3] lib/string_helpers et al.: Change " Andy Shevchenko
@ 2023-04-05 14:24 ` Greg Kroah-Hartman
2023-04-05 14:38 ` Andy Shevchenko
0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2023-04-05 14:24 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Kees Cook, Cezary Rojewski, linux-ext4, linux-kernel,
Theodore Ts'o, Jan Kara, Rafael J. Wysocki
On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> > It's more convenient to have strreplace() to return the pointer to
> > the string itself. This will help users to make their code better.
> >
> > The patch 1 kills the only user of the returned value of strreplace(),
> > Patch 2 converts the return value of strreplace(). And patch 3 shows
> > how it may be useful. That said, the series can be routed via fs tree,
> > with or without the last patch.
>
> Since there are no comments, who can apply this (patches 1 and 2)?
> Greg, are you fine with the kobject change?
Sure, want me to take them all through my driver-core tree?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()
2023-04-05 14:24 ` Greg Kroah-Hartman
@ 2023-04-05 14:38 ` Andy Shevchenko
2023-04-06 2:58 ` Kees Cook
0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2023-04-05 14:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Cezary Rojewski, linux-ext4, linux-kernel,
Theodore Ts'o, Jan Kara, Rafael J. Wysocki
On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> > > It's more convenient to have strreplace() to return the pointer to
> > > the string itself. This will help users to make their code better.
> > >
> > > The patch 1 kills the only user of the returned value of strreplace(),
> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
> > > how it may be useful. That said, the series can be routed via fs tree,
> > > with or without the last patch.
> >
> > Since there are no comments, who can apply this (patches 1 and 2)?
> > Greg, are you fine with the kobject change?
>
> Sure, want me to take them all through my driver-core tree?
Fine by me! Dunno about others. Kees?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()
2023-04-05 14:38 ` Andy Shevchenko
@ 2023-04-06 2:58 ` Kees Cook
2023-06-05 14:04 ` Andy Shevchenko
0 siblings, 1 reply; 12+ messages in thread
From: Kees Cook @ 2023-04-06 2:58 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Kees Cook, Cezary Rojewski, linux-ext4, linux-kernel,
Theodore Ts'o, Jan Kara, Rafael J. Wysocki
On April 5, 2023 7:38:40 AM PDT, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
>> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
>> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
>> > > It's more convenient to have strreplace() to return the pointer to
>> > > the string itself. This will help users to make their code better.
>> > >
>> > > The patch 1 kills the only user of the returned value of strreplace(),
>> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
>> > > how it may be useful. That said, the series can be routed via fs tree,
>> > > with or without the last patch.
>> >
>> > Since there are no comments, who can apply this (patches 1 and 2)?
>> > Greg, are you fine with the kobject change?
>>
>> Sure, want me to take them all through my driver-core tree?
>
>Fine by me! Dunno about others. Kees?
Yeah, that's cool by me. :)
--
Kees Cook
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()
2023-04-06 2:58 ` Kees Cook
@ 2023-06-05 14:04 ` Andy Shevchenko
2023-06-05 16:57 ` Greg Kroah-Hartman
0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2023-06-05 14:04 UTC (permalink / raw)
To: Kees Cook
Cc: Greg Kroah-Hartman, Kees Cook, Cezary Rojewski, linux-ext4,
linux-kernel, Theodore Ts'o, Jan Kara, Rafael J. Wysocki
On Wed, Apr 05, 2023 at 07:58:40PM -0700, Kees Cook wrote:
> On April 5, 2023 7:38:40 AM PDT, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
> >> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> >> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> >> > > It's more convenient to have strreplace() to return the pointer to
> >> > > the string itself. This will help users to make their code better.
> >> > >
> >> > > The patch 1 kills the only user of the returned value of strreplace(),
> >> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
> >> > > how it may be useful. That said, the series can be routed via fs tree,
> >> > > with or without the last patch.
> >> >
> >> > Since there are no comments, who can apply this (patches 1 and 2)?
> >> > Greg, are you fine with the kobject change?
> >>
> >> Sure, want me to take them all through my driver-core tree?
> >
> >Fine by me! Dunno about others. Kees?
>
> Yeah, that's cool by me. :)
Greg, does this slip through the cracks?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()
2023-06-05 14:04 ` Andy Shevchenko
@ 2023-06-05 16:57 ` Greg Kroah-Hartman
2023-06-05 17:06 ` Andy Shevchenko
0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2023-06-05 16:57 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Kees Cook, Kees Cook, Cezary Rojewski, linux-ext4, linux-kernel,
Theodore Ts'o, Jan Kara, Rafael J. Wysocki
On Mon, Jun 05, 2023 at 05:04:43PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 05, 2023 at 07:58:40PM -0700, Kees Cook wrote:
> > On April 5, 2023 7:38:40 AM PDT, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > >On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
> > >> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> > >> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> > >> > > It's more convenient to have strreplace() to return the pointer to
> > >> > > the string itself. This will help users to make their code better.
> > >> > >
> > >> > > The patch 1 kills the only user of the returned value of strreplace(),
> > >> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
> > >> > > how it may be useful. That said, the series can be routed via fs tree,
> > >> > > with or without the last patch.
> > >> >
> > >> > Since there are no comments, who can apply this (patches 1 and 2)?
> > >> > Greg, are you fine with the kobject change?
> > >>
> > >> Sure, want me to take them all through my driver-core tree?
> > >
> > >Fine by me! Dunno about others. Kees?
> >
> > Yeah, that's cool by me. :)
>
> Greg, does this slip through the cracks?
It did. Can someone resend this?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] lib/string_helpers et al.: Change return value of strreplace()
2023-06-05 16:57 ` Greg Kroah-Hartman
@ 2023-06-05 17:06 ` Andy Shevchenko
0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2023-06-05 17:06 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Kees Cook, Kees Cook, Cezary Rojewski, linux-ext4, linux-kernel,
Theodore Ts'o, Jan Kara, Rafael J. Wysocki
On Mon, Jun 05, 2023 at 06:57:48PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Jun 05, 2023 at 05:04:43PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 05, 2023 at 07:58:40PM -0700, Kees Cook wrote:
> > > On April 5, 2023 7:38:40 AM PDT, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > >On Wed, Apr 05, 2023 at 04:24:42PM +0200, Greg Kroah-Hartman wrote:
> > > >> On Wed, Apr 05, 2023 at 04:34:31PM +0300, Andy Shevchenko wrote:
> > > >> > On Thu, Mar 23, 2023 at 02:37:01PM +0200, Andy Shevchenko wrote:
> > > >> > > It's more convenient to have strreplace() to return the pointer to
> > > >> > > the string itself. This will help users to make their code better.
> > > >> > >
> > > >> > > The patch 1 kills the only user of the returned value of strreplace(),
> > > >> > > Patch 2 converts the return value of strreplace(). And patch 3 shows
> > > >> > > how it may be useful. That said, the series can be routed via fs tree,
> > > >> > > with or without the last patch.
> > > >> >
> > > >> > Since there are no comments, who can apply this (patches 1 and 2)?
> > > >> > Greg, are you fine with the kobject change?
> > > >>
> > > >> Sure, want me to take them all through my driver-core tree?
> > > >
> > > >Fine by me! Dunno about others. Kees?
> > >
> > > Yeah, that's cool by me. :)
> >
> > Greg, does this slip through the cracks?
>
> It did. Can someone resend this?
Done as v3.
20230605170553.7835-1-andriy.shevchenko@linux.intel.com
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 12+ messages in thread