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

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

[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.

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 <gang.chen@asianux.com>
---
 lib/vsprintf.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 739a363..b153768 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -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);
-- 
1.7.7.6

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

[George Spelvin]

> 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.

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

[Chen Gang]

On 07/18/2013 02:42 PM, George Spelvin wrote:
>> 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);
> 

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

> 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.
-- 
Chen Gang

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

[Al Viro]

On Thu, Jul 18, 2013 at 02:28:49PM +0800, Chen Gang wrote:
> 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.

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

[Chen Gang]

On 07/18/2013 02:49 PM, Al Viro wrote:
> On Thu, Jul 18, 2013 at 02:28:49PM +0800, Chen Gang wrote:
>> > 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.

-- 
Chen Gang