[PATCH iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

[PATCH iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

[James Cowgill]

hashlimit was using "%lu" in a lot of printf format specifiers to print
64-bit integers. This is incorrect on 32-bit architectures because
"long int" is 32-bits there. On MIPS, it was causing iptables to
segfault when printing these integers.

Fix by using the correct format specifier.

Signed-off-by: James Cowgill <James.Cowgill@imgtec.com>
---
 extensions/libxt_hashlimit.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/extensions/libxt_hashlimit.c b/extensions/libxt_hashlimit.c
index 52fc4fa..63cf903 100644
--- a/extensions/libxt_hashlimit.c
+++ b/extensions/libxt_hashlimit.c
@@ -12,9 +12,9 @@
  */
 #define _BSD_SOURCE 1
 #define _ISOC99_SOURCE 1
+#include <inttypes.h>
 #include <math.h>
 #include <stdbool.h>
-#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
@@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision)
 	if (v > max)
 		xtables_error(PARAMETER_PROBLEM, "bad value for option "
 			"\"--hashlimit-burst\", value \"%s\" too large "
-				"(max %lumb).", burst, max/1024/1024);
+				"(max %" PRIu64 "mb).", burst, max/1024/1024);
 	return v;
 }
 
@@ -285,8 +285,8 @@ static bool parse_bytes(const char *rate, void *val, struct hashlimit_mt_udata *
 	tmp = (uint64_t) r * factor;
 	if (tmp > max)
 		xtables_error(PARAMETER_PROBLEM,
-			"Rate value too large \"%llu\" (max %lu)\n",
-					(unsigned long long)tmp, max);
+			"Rate value too large \"%" PRIu64 "\" (max %" PRIu64 ")\n",
+					tmp, max);
 
 	tmp = bytes_to_cost(tmp);
 	if (tmp == 0)
@@ -557,7 +557,7 @@ static void hashlimit_mt_check_v1(struct xt_fcheck_call *cb)
 		if (cb->xflags & F_BURST) {
 			if (info->cfg.burst < cost_to_bytes(info->cfg.avg))
 				xtables_error(PARAMETER_PROBLEM,
-					"burst cannot be smaller than %lub", cost_to_bytes(info->cfg.avg));
+					"burst cannot be smaller than %" PRIu64 "b", cost_to_bytes(info->cfg.avg));
 
 			burst = info->cfg.burst;
 			burst /= cost_to_bytes(info->cfg.avg);
@@ -587,7 +587,7 @@ static void hashlimit_mt_check(struct xt_fcheck_call *cb)
 		if (cb->xflags & F_BURST) {
 			if (info->cfg.burst < cost_to_bytes(info->cfg.avg))
 				xtables_error(PARAMETER_PROBLEM,
-					"burst cannot be smaller than %lub", cost_to_bytes(info->cfg.avg));
+					"burst cannot be smaller than %" PRIu64 "b", cost_to_bytes(info->cfg.avg));
 
 			burst = info->cfg.burst;
 			burst /= cost_to_bytes(info->cfg.avg);
@@ -631,7 +631,7 @@ static uint32_t print_rate(uint32_t period, int revision)
             || _rates[i].mult/period < _rates[i].mult%period)
 			break;
 
-	printf(" %lu/%s", _rates[i-1].mult / period, _rates[i-1].name);
+	printf(" %" PRIu64 "/%s", _rates[i-1].mult / period, _rates[i-1].name);
 	/* return in msec */
 	return _rates[i-1].mult / scale * 1000;
 }
-- 
2.11.0

Re: [PATCH iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

[Arturo Borrero Gonzalez]

On 7 April 2017 at 13:47, James Cowgill <James.Cowgill@imgtec.com> wrote:
> hashlimit was using "%lu" in a lot of printf format specifiers to print
> 64-bit integers. This is incorrect on 32-bit architectures because
> "long int" is 32-bits there. On MIPS, it was causing iptables to
> segfault when printing these integers.
>
> Fix by using the correct format specifier.
>
> Signed-off-by: James Cowgill <James.Cowgill@imgtec.com>
> ---
>  extensions/libxt_hashlimit.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>

For reference, this is related to debian bug #859775 [0]

[0] https://bugs.debian.org/859775

Re: [PATCH iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

[Pablo Neira Ayuso]

On Fri, Apr 07, 2017 at 12:47:29PM +0100, James Cowgill wrote:
> hashlimit was using "%lu" in a lot of printf format specifiers to print
> 64-bit integers. This is incorrect on 32-bit architectures because
> "long int" is 32-bits there. On MIPS, it was causing iptables to
> segfault when printing these integers.
> 
> Fix by using the correct format specifier.

One comment below...

> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision)
>  	if (v > max)
>  		xtables_error(PARAMETER_PROBLEM, "bad value for option "
>  			"\"--hashlimit-burst\", value \"%s\" too large "
> -				"(max %lumb).", burst, max/1024/1024);
> +				"(max %" PRIu64 "mb).", burst, max/1024/1024);
                                        ^      ^

I can remove this whitespaces, right? I'm looking at other spots in
the iptables code and I would like to keep this consistent.

No need to resent the patch, I can just mangle this here and apply.

Thanks!

Re: [PATCH iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

[James Cowgill]

On 08/04/17 21:29, Pablo Neira Ayuso wrote:
> On Fri, Apr 07, 2017 at 12:47:29PM +0100, James Cowgill wrote:
>> hashlimit was using "%lu" in a lot of printf format specifiers to print
>> 64-bit integers. This is incorrect on 32-bit architectures because
>> "long int" is 32-bits there. On MIPS, it was causing iptables to
>> segfault when printing these integers.
>>
>> Fix by using the correct format specifier.
> 
> One comment below...
> 
>> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision)
>>  	if (v > max)
>>  		xtables_error(PARAMETER_PROBLEM, "bad value for option "
>>  			"\"--hashlimit-burst\", value \"%s\" too large "
>> -				"(max %lumb).", burst, max/1024/1024);
>> +				"(max %" PRIu64 "mb).", burst, max/1024/1024);
>                                         ^      ^
> 
> I can remove this whitespaces, right? I'm looking at other spots in
> the iptables code and I would like to keep this consistent.
> 
> No need to resent the patch, I can just mangle this here and apply.

Yes removing the spaces around PRIu64 should be fine.

James

Re: [PATCH iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

[Jan Engelhardt]

On Saturday 2017-04-08 22:29, Pablo Neira Ayuso wrote:
>> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision)
>>  	if (v > max)
>>  		xtables_error(PARAMETER_PROBLEM, "bad value for option "
>>  			"\"--hashlimit-burst\", value \"%s\" too large "
>> -				"(max %lumb).", burst, max/1024/1024);
>> +				"(max %" PRIu64 "mb).", burst, max/1024/1024);
>                                        ^      ^
>
>I can remove this whitespaces, right?

With my distro hat on:

Clumping these together like "foo"BAR"baz" has already caused compile failures
in the broader scope of distributions (thousands of packages) because languages
introduced new tokenization rules. Admittedly, this occurred in C++ (namely,
user-defined string literals), but it does show that tokens which logically are
separate should stay separate.

Re: [PATCH iptables] extensions: libxt_hashlimit: fix 64-bit printf formats

[Pablo Neira Ayuso]

On Mon, Apr 10, 2017 at 05:59:07PM +0200, Jan Engelhardt wrote:
> 
> On Saturday 2017-04-08 22:29, Pablo Neira Ayuso wrote:
> >> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision)
> >>  	if (v > max)
> >>  		xtables_error(PARAMETER_PROBLEM, "bad value for option "
> >>  			"\"--hashlimit-burst\", value \"%s\" too large "
> >> -				"(max %lumb).", burst, max/1024/1024);
> >> +				"(max %" PRIu64 "mb).", burst, max/1024/1024);
> >                                        ^      ^
> >
> >I can remove this whitespaces, right?
> 
> With my distro hat on:
> 
> Clumping these together like "foo"BAR"baz" has already caused compile failures
> in the broader scope of distributions (thousands of packages) because languages
> introduced new tokenization rules. Admittedly, this occurred in C++ (namely,
> user-defined string literals), but it does show that tokens which logically are
> separate should stay separate.

Existing code doesn't add space, so I would like we don't introduce
inconsistencies.