Messages in this thread Patch in this message | | | From | Gavrilov Ilia <> | Subject | Re: [PATCH] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function. | Date | Wed, 11 Jan 2023 11:18:52 +0000 |
| |
On 1/11/23 13:19, Simon Horman wrote: > Hi Gavrilov, > > On Mon, Jan 09, 2023 at 11:54:02AM +0000, Gavrilov Ilia wrote: >> When first_ip is 0, last_ip is 0xFFFFFFF, and netmask is 31, the value of >> an arithmetic expression 2 << (netmask - mask_bits - 1) is subject >> to overflow due to a failure casting operands to a larger data type >> before performing the arithmetic. >> >> Note that it's harmless since the value will be checked at the next step. > > Do you mean 0xFFFFFFFF (8 rather than 8 'F's) ? > If so, I agree with this patch. >
Yes, it's my typo. I meant 0xFFFFFFFF.
>> Found by InfoTeCS on behalf of Linux Verification Center >> (linuxtesting.org) with SVACE. >> >> Fixes: b9fed748185a ("netfilter: ipset: Check and reject crazy /0 input parameters") >> Signed-off-by: Ilia.Gavrilov <Ilia.Gavrilov@infotecs.ru> >> --- >> net/netfilter/ipset/ip_set_bitmap_ip.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c b/net/netfilter/ipset/ip_set_bitmap_ip.c >> index a8ce04a4bb72..b8f0fb37378f 100644 >> --- a/net/netfilter/ipset/ip_set_bitmap_ip.c >> +++ b/net/netfilter/ipset/ip_set_bitmap_ip.c >> @@ -309,7 +309,7 @@ bitmap_ip_create(struct net *net, struct ip_set *set, struct nlattr *tb[], >> >> pr_debug("mask_bits %u, netmask %u\n", mask_bits, netmask); >> hosts = 2 << (32 - netmask - 1); > > I think that hosts also overflows, in the case you have described. > Although it also doesn't matter for the same reason you state. > But from a correctness point of view perhaps it should also be addressed? >
As for 'hosts', the expression "2 << (32 - netmask - 1)" is also subject to overflow, but the type of the variable 'hosts' is u32, and the type casting gives the correct result. But I will fix it for correctness.
Thank you for review. I will change that in V2.
Ilia.
>> - elements = 2 << (netmask - mask_bits - 1); >> + elements = 2UL << (netmask - mask_bits - 1); >> } >> if (elements > IPSET_BITMAP_MAX_RANGE + 1) >> return -IPSET_ERR_BITMAP_RANGE_SIZE; >> -- >> 2.30.2 >>
| |