Skip to contentSkip to conversation

lib/vsprintf.c: fix the incorrect return value of vsnprintf()

PATCH In review Chen Gang 3 participants 5 messages Jul 18, 2013

Subsystems (2)
Library code Vsprintf
lore
Share & tools
lore

Chen Gang

author
#0 lore raw

PATCH 1/1 lib/vsprintf.c: fix the incorrect return value of vsnprintf()

When "str >= end", necessary to reset 'str' to "end - 1", or the return value will be larger than the real one, the callers which depend on the return value, may cause memory overflow.

When for copying constant string, 'str' is point to destination buffer, so it need add 'copy' (for destination buffer), not 'read' (for source buffer).

Also add white space between variable and operator to make code cleaner.

Signed-off-by Chen Gang <[email protected]>
---
 lib/vsprintf.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
lib/vsprintf.c +5 -3
index 739a363..b153768 100644
@@ -1542,7 +1542,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 					copy = end - str;
 				memcpy(str, old_fmt, copy);
 			}
-			str += read;
+			str += copy;
 			break;
 		}
 

@@ -1662,12 +1662,14 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
 	if (size > 0) {
 		if (str < end)
 			*str = '\0';
-		else
+		else {
 			end[-1] = '\0';
+			str = end - 1;
+		}
 	}
 
 	/* the trailing null byte doesn't count towards the total */
-	return str-buf;
+	return str - buf;
 
 }
 EXPORT_SYMBOL(vsnprintf);
Signature · 1 line
1.7.7.6
#1 lore raw
Quoted 3 lines

When "str >= end", necessary to reset 'str' to "end - 1", or the return value will be larger than the real one, the callers which depend on the return value, may cause memory overflow.

NAK. This is the documented (by both the function itself and the ANSI/ISO C standard) and desired return value: the number of bytes that *would* have been in the output string if the buffer were large enough. In particular, it is common to do:

size = vsnprintf(NULL, 0, fmt, args) + 1;
p = malloc(size, GFP_KERNEL);
vsnprintf(p, size, fmt, args);

You want vscnprintf. If you have a caller that needs the *actual* number of bytes written, use that.

author
#2 lore raw
Quoted 12 lines · George Spelvin
Quoted 3 lines

When "str >= end", necessary to reset 'str' to "end - 1", or the return

Quoted 2 lines

value will be larger than the real one, the callers which depend on the return value, may cause memory overflow.

NAK. This is the documented (by both the function itself and the ANSI/ISO C standard) and desired return value: the number of bytes that *would* have been in the output string if the buffer were large enough. In particular, it is common to do:

size = vsnprintf(NULL, 0, fmt, args) + 1;
p = malloc(size, GFP_KERNEL);
vsnprintf(p, size, fmt, args);

OK, it is my fault, thank you very much.

Quoted 4 lines

You want vscnprintf. If you have a caller that needs the *actual* number of bytes written, use that.

Yeah, my another patch need use vscnprintf() instead of vsnprintf(), thanks again.

Thanks.

Signature · 1 line
Chen Gang
#3 lore raw
In reply to Chen Gang
Quoted 3 lines · Chen Gang

When "str >= end", necessary to reset 'str' to "end - 1", or the return value will be larger than the real one, the callers which depend on the return value, may cause memory overflow.

You do realize that snprintf(s, 1, "abc") should return 3, not 1? The goal off snprintf() is _not_ just to truncate the output; return value tells how much should the buffer had been to fit the whole thing.

author
#4 lore raw
Quoted 9 lines · Al Viro
Quoted 3 lines · Chen Gang
Quoted 3 lines

When "str >= end", necessary to reset 'str' to "end - 1", or the return value will be larger than the real one, the callers which depend on the return value, may cause memory overflow.

You do realize that snprintf(s, 1, "abc") should return 3, not 1? The goal off snprintf() is _not_ just to truncate the output; return value tells how much should the buffer had been to fit the whole thing.

It is my fault, thanks.

Signature · 1 line
Chen Gang