All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] lib/vsprintf.c: fix the incorrect return value of vsnprintf()
@ 2013-07-18  6:28 Chen Gang
  2013-07-18  6:42 ` George Spelvin
  2013-07-18  6:49 ` Al Viro
  0 siblings, 2 replies; 5+ messages in thread
From: Chen Gang @ 2013-07-18  6:28 UTC (permalink / raw)
  To: linux, Jiri Kosina, andriy.shevchenko, andrei.emeltchenko
  Cc: Andrew Morton, linux-kernel

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

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

* Re: [PATCH] lib/vsprintf.c: fix the incorrect return value of vsnprintf()
  2013-07-18  6:28 [PATCH] lib/vsprintf.c: fix the incorrect return value of vsnprintf() Chen Gang
@ 2013-07-18  6:42 ` George Spelvin
  2013-07-18  6:50   ` Chen Gang
  2013-07-18  6:49 ` Al Viro
  1 sibling, 1 reply; 5+ messages in thread
From: George Spelvin @ 2013-07-18  6:42 UTC (permalink / raw)
  To: andrei.emeltchenko, andriy.shevchenko, gang.chen, jkosina, linux
  Cc: akpm, linux-kernel

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

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

* Re: [PATCH] lib/vsprintf.c: fix the incorrect return value of vsnprintf()
  2013-07-18  6:28 [PATCH] lib/vsprintf.c: fix the incorrect return value of vsnprintf() Chen Gang
  2013-07-18  6:42 ` George Spelvin
@ 2013-07-18  6:49 ` Al Viro
  2013-07-18  6:52   ` Chen Gang
  1 sibling, 1 reply; 5+ messages in thread
From: Al Viro @ 2013-07-18  6:49 UTC (permalink / raw)
  To: Chen Gang
  Cc: linux, Jiri Kosina, andriy.shevchenko, andrei.emeltchenko,
	Andrew Morton, linux-kernel

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.

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

* Re: [PATCH] lib/vsprintf.c: fix the incorrect return value of vsnprintf()
  2013-07-18  6:42 ` George Spelvin
@ 2013-07-18  6:50   ` Chen Gang
  0 siblings, 0 replies; 5+ messages in thread
From: Chen Gang @ 2013-07-18  6:50 UTC (permalink / raw)
  To: George Spelvin
  Cc: andrei.emeltchenko, andriy.shevchenko, jkosina, akpm, linux-kernel

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

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

* Re: [PATCH] lib/vsprintf.c: fix the incorrect return value of vsnprintf()
  2013-07-18  6:49 ` Al Viro
@ 2013-07-18  6:52   ` Chen Gang
  0 siblings, 0 replies; 5+ messages in thread
From: Chen Gang @ 2013-07-18  6:52 UTC (permalink / raw)
  To: Al Viro
  Cc: linux, Jiri Kosina, andriy.shevchenko, andrei.emeltchenko,
	Andrew Morton, linux-kernel

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

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

end of thread, other threads:[~2013-07-18  6:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-18  6:28 [PATCH] lib/vsprintf.c: fix the incorrect return value of vsnprintf() Chen Gang
2013-07-18  6:42 ` George Spelvin
2013-07-18  6:50   ` Chen Gang
2013-07-18  6:49 ` Al Viro
2013-07-18  6:52   ` Chen Gang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.