utils: fix buffer reallocation of nft_fprinft()
PATCH 1/1 utils: fix buffer reallocation of nft_fprinft()
When _snprintf() reports it would print n characters, that n doesn't include the trailing \0 that snprintf adds.
Thus, we need to [re]allocate n+1 characters.
While at it, change the reallocation trigger. If the length of the buffer we used is equals to the expanded string length, the output has been truncated. In other words, if ret == bufsiz, then the trailing \0 is missing.
--- src/utils.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
src/utils.c +4 -3
@@ -195,12 +195,13 @@ int nft_fprintf(FILE *fp, void *obj, uint32_t type, uint32_t flags,int ret; ret =snprintf_cb (buf, bufsiz, obj, type, flags); -if (ret >NFT_SNPRINTF_BUFSIZ ) { - buf =calloc (1 , ret); +if (ret >=NFT_SNPRINTF_BUFSIZ ) { + bufsiz = ret +1 ; + + buf =calloc (1 , bufsiz);if (buf ==NULL )return -1 ; - bufsiz = ret; ret =snprintf_cb (buf, bufsiz, obj, type, flags); }
Pablo Neira Ayuso
maintainerHi Arturo,
Quoted 19 lines · Arturo Borrero Gonzalez
When
_snprintf()reports it would print n characters, that n doesn't include the trailing \0 that snprintf adds.Thus, we need to [re]allocate n+1 characters.
While at it, change the reallocation trigger. If the length of the buffer we used is equals to the expanded string length, the output has been truncated. In other words, if ret == bufsiz, then the trailing \0 is missing.
Signed-off-by Arturo Borrero Gonzalez <[email protected]>--- src/utils.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)src/utils.c +0
-0
src/utils.c int nft_fprintf(FILE *fp, void *obj, uint32_t type, uint32_t flags, · line 197
Quoted 3 lines
src/utils.c +0
-0@@ -195,12 +195,13 @@ int nft_fprintf(FILE *fp, void *obj, uint32_t type, uint32_t flags,int ret; ret =snprintf_cb (buf, bufsiz, obj, type, flags);
I think we should also check here if snprintf returns -1 now that you have fixed the SNPRINTF_ macro.
src/utils.c int nft_fprintf(FILE *fp, void *obj, uint32_t type, uint32_t flags, · line 201
Quoted 6 lines
src/utils.c +4
-2-if (ret >NFT_SNPRINTF_BUFSIZ ) { - buf =calloc (1 , ret); +if (ret >=NFT_SNPRINTF_BUFSIZ ) { + bufsiz = ret +1 ; + + buf =calloc (1 , bufsiz);
You can use malloc instead. Just make sure that the string is always nul-terminated before printing, something like:
bufsiz = ret + 1;
buf = malloc(1, bufsiz);
if (buf == NULL)
return -1;
ret = snprintf(...
if (ret < 0)
...
}
buf[ret] = '\0';
... = fprintf(...Thanks.
Quoted 16 lines · Pablo Neira Ayuso
You can use malloc instead. Just make sure that the string is always nul-terminated before printing, something like:
bufsiz = ret + 1; buf = malloc(1, bufsiz); if (buf == NULL) return -1; ret = snprintf(... if (ret < 0) ... } buf[ret] = '\0'; ... = fprintf(...
From my man pages, I understand that snprintf() null-terminate the string. This seems a bit redundant.
I think it's safe to don't include calloc() neither buf[ret] = '\0'.
I'm resending this patch with this assumption and your other request.
regards.
Signature · 5 lines
Arturo Borrero González -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso
maintainerQuoted 14 lines · Arturo Borrero Gonzalez
Quoted 13 lines · Pablo Neira Ayuso
You can use malloc instead. Just make sure that the string is always nul-terminated before printing, something like:
bufsiz = ret + 1; buf = malloc(1, bufsiz); if (buf == NULL) return -1; ret = snprintf(... if (ret < 0) ... }
Quoted 6 lines
Quoted 3 lines
buf[ret] = '\0'; ... = fprintf(...From my man pages, I understand that
snprintf()null-terminate the string. This seems a bit redundant.
If the amount of written bytes is smaller than the buffer size, snprintf always nul-terminate it. But if the amount of bytes returned equals the buffer size, then you have to explicitly nul-terminate the string.
Quoted 1 line
I think it's safe to don't include
calloc()neither buf[ret] = '\0'.
Right, if you use a buffer size of ret + 1, then you can guarantee that snprintf always have room to append the nul-termination, as the returned value will be ret at maximum.
Quoted 1 line
I'm resending this patch with this assumption and your other request.
OK, thanks.