[PATCH] firmware/memmap: use scnprintf() in show funcs

[PATCH] firmware/memmap: use scnprintf() in show funcs

[Pranav Tyagi]

Replace all snprintf() instances with scnprintf(). snprintf() returns
the number of bytes that would have been written had there been enough
space. For sysfs attributes, snprintf() should not be used for the
show() method. Instead use scnprintf() which returns the number of bytes
actually written.

Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
---
 drivers/firmware/memmap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/memmap.c b/drivers/firmware/memmap.c
index 55b9cfad8a04..fef18f598ff8 100644
--- a/drivers/firmware/memmap.c
+++ b/drivers/firmware/memmap.c
@@ -369,19 +369,19 @@ int __meminit firmware_map_remove(u64 start, u64 end, const char *type)
 
 static ssize_t start_show(struct firmware_map_entry *entry, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "0x%llx\n",
+	return scnprintf(buf, PAGE_SIZE, "0x%llx\n",
 		(unsigned long long)entry->start);
 }
 
 static ssize_t end_show(struct firmware_map_entry *entry, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "0x%llx\n",
+	return scnprintf(buf, PAGE_SIZE, "0x%llx\n",
 		(unsigned long long)entry->end);
 }
 
 static ssize_t type_show(struct firmware_map_entry *entry, char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%s\n", entry->type);
+	return scnprintf(buf, PAGE_SIZE, "%s\n", entry->type);
 }
 
 static inline struct memmap_attribute *to_memmap_attr(struct attribute *attr)
-- 
2.49.0

Re: [PATCH] firmware/memmap: use scnprintf() in show funcs

[Greg KH]

On Fri, Jun 13, 2025 at 07:14:49PM +0530, Pranav Tyagi wrote:
> Replace all snprintf() instances with scnprintf(). snprintf() returns
> the number of bytes that would have been written had there been enough
> space. For sysfs attributes, snprintf() should not be used for the
> show() method. Instead use scnprintf() which returns the number of bytes
> actually written.

for sysfs attributes, sysfs_emit() should be used instead.  Why not do
that here?

But what is wrong with the current code?  Is it not working properly?

thanks,

greg k-h

Re: [PATCH] firmware/memmap: use scnprintf() in show funcs

[Pranav Tyagi]

On Tue, Jul 1, 2025 at 3:57 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jun 13, 2025 at 07:14:49PM +0530, Pranav Tyagi wrote:
> > Replace all snprintf() instances with scnprintf(). snprintf() returns
> > the number of bytes that would have been written had there been enough
> > space. For sysfs attributes, snprintf() should not be used for the
> > show() method. Instead use scnprintf() which returns the number of bytes
> > actually written.
>
> for sysfs attributes, sysfs_emit() should be used instead.  Why not do
> that here?
>
> But what is wrong with the current code?  Is it not working properly?
>
> thanks,
>
> greg k-h'

I had sent this patch earlier along with a few others of similar
nature. The code in question was working correctly. However, the
intent was to perform cleanups based on documentation guidance.

Specifically, for sysfs attributes, the documentation discourages
using snprintf() in the show() method since it returns the
number of bytes that would have been written, not the number
actually written. This can potentially lead to buffer overflows.

You had previously pointed out that either sysfs_emit() should
be used or the code should be left unchanged if it is working
correctly. To avoid unnecessary churn, I decided to drop those
( including this one ) patches entirely.

Regards
Pranav Tyagi