On Sat, Mar 23, 2024 at 06:06:41AM +0300, Vitaly Chikunov wrote: > Frequently when addr/mask is printed xtables_ipaddr_to_numeric and > xtables_ipmask_to_numeric are called together in one printf call but > xtables_ipmask_to_numeric internally calls xtables_ipaddr_to_numeric > which prints into the same static buffer causing buffer to be > overwritten and addr/mask incorrectly printed in such call scenarios. > > Make xtables_ipaddr_to_numeric to use two static buffers rotating their > use. This simplistic approach will leave ABI not changed and cover all > such use cases. Additional thoughts are: the call order of these two function is important in will it override buf or not. Order of evaluation of function arguments is unspecified[1] and compile time dependent. It seems that on non-x86 architectures, sometime it could be different from left-to-right. That's why this bug was unnoticed for so much time. [1] https://en.cppreference.com/w/c/language/eval_order Thanks, > > Interestingly, testing stumbled over this only on non-x86 architectures. > Error message: > > extensions/libebt_arp.t: ERROR: line 11 (cannot find: ebtables -I INPUT -p ARP --arp-ip-src ! 1.2.3.4/255.0.255.255) > > Signed-off-by: Vitaly Chikunov <vt@xxxxxxxxxxxx> > Cc: Jan Engelhardt <jengelh@xxxxxxx> > Cc: Gleb Fotengauer-Malinovskiy <glebfm@xxxxxxxxxxxx> > --- > libxtables/xtables.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libxtables/xtables.c b/libxtables/xtables.c > index 748a50fc..16a0640d 100644 > --- a/libxtables/xtables.c > +++ b/libxtables/xtables.c > @@ -1505,7 +1505,9 @@ void xtables_param_act(unsigned int status, const char *p1, ...) > > const char *xtables_ipaddr_to_numeric(const struct in_addr *addrp) > { > - static char buf[16]; > + static char abuf[2][16]; > + static int bufnum = 0; > + char *buf = abuf[++bufnum & 1]; > const unsigned char *bytep = (const void *)&addrp->s_addr; > > sprintf(buf, "%u.%u.%u.%u", bytep[0], bytep[1], bytep[2], bytep[3]); > -- > 2.42.1
