diff mbox

[libnftnl,v2] internal: fix SNPRINTF_BUFFER_SIZE macro

Message ID 20140513070735.16866.22056.stgit@nfdev.cica.es
State Accepted
Headers show

Commit Message

Arturo Borrero May 13, 2014, 7:08 a.m. UTC
We need to store in 'offset' the complete amount of characters as returned
from _snprintf. The value means how many characters long needs the buffer to be
in order to store the corresponding string expansion.

Before this patch, in cases where the buffer is smaller than the
expansion, then ret > len, and therefore ret = len.
So when incrementing offset, we do it with a wrong value.

All previous versions of libnftnl are unable to handle this situations: small
buffers (or long string expansion).

BTW, if a caller must reallocate a buffer to the returned value of snprintf, it
should be ret + 1.

While at it, let's add a check to know if the last snprintf call failed.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
v2: return if ret < 0. Increment size later.

 src/internal.h |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Pablo Neira Ayuso May 13, 2014, 3:24 p.m. UTC | #1
On Tue, May 13, 2014 at 09:08:15AM +0200, Arturo Borrero Gonzalez wrote:
> We need to store in 'offset' the complete amount of characters as returned
> from _snprintf. The value means how many characters long needs the buffer to be
> in order to store the corresponding string expansion.
> 
> Before this patch, in cases where the buffer is smaller than the
> expansion, then ret > len, and therefore ret = len.
> So when incrementing offset, we do it with a wrong value.
> 
> All previous versions of libnftnl are unable to handle this situations: small
> buffers (or long string expansion).
> 
> BTW, if a caller must reallocate a buffer to the returned value of snprintf, it
> should be ret + 1.
> 
> While at it, let's add a check to know if the last snprintf call failed.

Also applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/src/internal.h b/src/internal.h
index 6595e70..b06f166 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -183,10 +183,12 @@  struct nft_set_elem {
 };
 
 #define SNPRINTF_BUFFER_SIZE(ret, size, len, offset)	\
-	size += ret;					\
+	if (ret < 0)					\
+		return ret;				\
+	offset += ret;					\
 	if (ret > len)					\
 		ret = len;				\
-	offset += ret;					\
+	size += ret;					\
 	len -= ret;
 
 #define div_round_up(n, d)	(((n) + (d) - 1) / (d))