LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] lib/string_helpers et al.: Change return value of strreplace()
@ 2023-06-05 17:05 Andy Shevchenko
  2023-06-05 17:05 ` [PATCH v3 1/3] jbd2: Avoid printing outside the boundary of the buffer Andy Shevchenko
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Andy Shevchenko @ 2023-06-05 17:05 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 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.

In v3:
- rebased on top of latest Linux Next

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(-)

-- 
2.40.0.1.gaa8946217a0b


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/3] jbd2: Avoid printing outside the boundary of the buffer
  2023-06-05 17:05 [PATCH v3 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
@ 2023-06-05 17:05 ` Andy Shevchenko
  2023-06-05 17:05 ` [PATCH v3 2/3] lib/string_helpers: Change returned value of the strreplace() Andy Shevchenko
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2023-06-05 17:05 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".

Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 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] 5+ messages in thread

* [PATCH v3 2/3] lib/string_helpers: Change returned value of the strreplace()
  2023-06-05 17:05 [PATCH v3 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
  2023-06-05 17:05 ` [PATCH v3 1/3] jbd2: Avoid printing outside the boundary of the buffer Andy Shevchenko
@ 2023-06-05 17:05 ` Andy Shevchenko
  2023-06-05 17:05 ` [PATCH v3 3/3] kobject: Use return value of strreplace() Andy Shevchenko
  2023-06-05 22:31 ` [PATCH v3 0/3] lib/string_helpers et al.: Change " Kees Cook
  3 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2023-06-05 17:05 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 c062c581a98b..dbfc66400050 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] 5+ messages in thread

* [PATCH v3 3/3] kobject: Use return value of strreplace()
  2023-06-05 17:05 [PATCH v3 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
  2023-06-05 17:05 ` [PATCH v3 1/3] jbd2: Avoid printing outside the boundary of the buffer Andy Shevchenko
  2023-06-05 17:05 ` [PATCH v3 2/3] lib/string_helpers: Change returned value of the strreplace() Andy Shevchenko
@ 2023-06-05 17:05 ` Andy Shevchenko
  2023-06-05 22:31 ` [PATCH v3 0/3] lib/string_helpers et al.: Change " Kees Cook
  3 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2023-06-05 17:05 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] 5+ messages in thread

* Re: [PATCH v3 0/3] lib/string_helpers et al.: Change return value of strreplace()
  2023-06-05 17:05 [PATCH v3 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
                   ` (2 preceding siblings ...)
  2023-06-05 17:05 ` [PATCH v3 3/3] kobject: Use return value of strreplace() Andy Shevchenko
@ 2023-06-05 22:31 ` Kees Cook
  3 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2023-06-05 22:31 UTC (permalink / raw
  To: linux-kernel, andriy.shevchenko, gregkh, linux-ext4,
	cezary.rojewski
  Cc: Kees Cook, tytso, jack, andy, rafael

On Mon, 5 Jun 2023 20:05:50 +0300, 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.
> 
> [...]

Applied to for-next/hardening, thanks!

[1/3] jbd2: Avoid printing outside the boundary of the buffer
      https://git.kernel.org/kees/c/7afb6d8fa81f
[2/3] lib/string_helpers: Change returned value of the strreplace()
      https://git.kernel.org/kees/c/d01a77afd6be
[3/3] kobject: Use return value of strreplace()
      https://git.kernel.org/kees/c/b2f10148ec1e

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-06-05 22:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-05 17:05 [PATCH v3 0/3] lib/string_helpers et al.: Change return value of strreplace() Andy Shevchenko
2023-06-05 17:05 ` [PATCH v3 1/3] jbd2: Avoid printing outside the boundary of the buffer Andy Shevchenko
2023-06-05 17:05 ` [PATCH v3 2/3] lib/string_helpers: Change returned value of the strreplace() Andy Shevchenko
2023-06-05 17:05 ` [PATCH v3 3/3] kobject: Use return value of strreplace() Andy Shevchenko
2023-06-05 22:31 ` [PATCH v3 0/3] lib/string_helpers et al.: Change " Kees Cook

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 read-only IMAP folder(s) and NNTP newsgroup(s).