* [PATCH] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function.
@ 2023-01-09 11:54 Gavrilov Ilia
2023-01-11 10:19 ` Simon Horman
0 siblings, 1 reply; 6+ messages in thread
From: Gavrilov Ilia @ 2023-01-09 11:54 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Jozsef Kadlecsik, Florian Westphal, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netfilter-devel@vger.kernel.org,
coreteam@netfilter.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org
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.
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);
- 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function.
2023-01-09 11:54 [PATCH] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function Gavrilov Ilia
@ 2023-01-11 10:19 ` Simon Horman
2023-01-11 11:18 ` Gavrilov Ilia
2023-01-11 11:57 ` [PATCH v2] " Gavrilov Ilia
0 siblings, 2 replies; 6+ messages in thread
From: Simon Horman @ 2023-01-11 10:19 UTC (permalink / raw)
To: Gavrilov Ilia
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
lvc-project@linuxtesting.org
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.
> 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?
> - 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function.
2023-01-11 10:19 ` Simon Horman
@ 2023-01-11 11:18 ` Gavrilov Ilia
2023-01-11 11:57 ` [PATCH v2] " Gavrilov Ilia
1 sibling, 0 replies; 6+ messages in thread
From: Gavrilov Ilia @ 2023-01-11 11:18 UTC (permalink / raw)
To: Simon Horman
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
lvc-project@linuxtesting.org
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
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function.
2023-01-11 10:19 ` Simon Horman
2023-01-11 11:18 ` Gavrilov Ilia
@ 2023-01-11 11:57 ` Gavrilov Ilia
2023-01-11 12:00 ` Simon Horman
1 sibling, 1 reply; 6+ messages in thread
From: Gavrilov Ilia @ 2023-01-11 11:57 UTC (permalink / raw)
To: Simon Horman
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
lvc-project@linuxtesting.org
When first_ip is 0, last_ip is 0xFFFFFFFF, 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.
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>
---
v2: Fix typo of the last_ip value in the description. Fix the expression for 'hosts'.
net/netfilter/ipset/ip_set_bitmap_ip.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c b/net/netfilter/ipset/ip_set_bitmap_ip.c
index a8ce04a4bb72..e4fa00abde6a 100644
--- a/net/netfilter/ipset/ip_set_bitmap_ip.c
+++ b/net/netfilter/ipset/ip_set_bitmap_ip.c
@@ -308,8 +308,8 @@ bitmap_ip_create(struct net *net, struct ip_set *set, struct nlattr *tb[],
return -IPSET_ERR_BITMAP_RANGE;
pr_debug("mask_bits %u, netmask %u\n", mask_bits, netmask);
- hosts = 2 << (32 - netmask - 1);
- elements = 2 << (netmask - mask_bits - 1);
+ hosts = 2U << (32 - netmask - 1);
+ elements = 2UL << (netmask - mask_bits - 1);
}
if (elements > IPSET_BITMAP_MAX_RANGE + 1)
return -IPSET_ERR_BITMAP_RANGE_SIZE;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function.
2023-01-11 11:57 ` [PATCH v2] " Gavrilov Ilia
@ 2023-01-11 12:00 ` Simon Horman
2023-01-11 18:10 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Simon Horman @ 2023-01-11 12:00 UTC (permalink / raw)
To: Gavrilov Ilia
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
lvc-project@linuxtesting.org
On Wed, Jan 11, 2023 at 11:57:39AM +0000, Gavrilov Ilia wrote:
> [You don't often get email from ilia.gavrilov@infotecs.ru. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> When first_ip is 0, last_ip is 0xFFFFFFFF, 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.
>
> 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>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
> ---
> v2: Fix typo of the last_ip value in the description. Fix the expression for 'hosts'.
> net/netfilter/ipset/ip_set_bitmap_ip.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c b/net/netfilter/ipset/ip_set_bitmap_ip.c
> index a8ce04a4bb72..e4fa00abde6a 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_ip.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_ip.c
> @@ -308,8 +308,8 @@ bitmap_ip_create(struct net *net, struct ip_set *set, struct nlattr *tb[],
> return -IPSET_ERR_BITMAP_RANGE;
>
> pr_debug("mask_bits %u, netmask %u\n", mask_bits, netmask);
> - hosts = 2 << (32 - netmask - 1);
> - elements = 2 << (netmask - mask_bits - 1);
> + hosts = 2U << (32 - netmask - 1);
> + elements = 2UL << (netmask - mask_bits - 1);
> }
> if (elements > IPSET_BITMAP_MAX_RANGE + 1)
> return -IPSET_ERR_BITMAP_RANGE_SIZE;
> --
> 2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function.
2023-01-11 12:00 ` Simon Horman
@ 2023-01-11 18:10 ` Pablo Neira Ayuso
0 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2023-01-11 18:10 UTC (permalink / raw)
To: Simon Horman
Cc: Gavrilov Ilia, Jozsef Kadlecsik, Florian Westphal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
lvc-project@linuxtesting.org
On Wed, Jan 11, 2023 at 01:00:53PM +0100, Simon Horman wrote:
> On Wed, Jan 11, 2023 at 11:57:39AM +0000, Gavrilov Ilia wrote:
> > [You don't often get email from ilia.gavrilov@infotecs.ru. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > When first_ip is 0, last_ip is 0xFFFFFFFF, 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.
> >
> > 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>
>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
Applied, thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-11 18:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-09 11:54 [PATCH] netfilter: ipset: Fix overflow before widen in the bitmap_ip_create() function Gavrilov Ilia
2023-01-11 10:19 ` Simon Horman
2023-01-11 11:18 ` Gavrilov Ilia
2023-01-11 11:57 ` [PATCH v2] " Gavrilov Ilia
2023-01-11 12:00 ` Simon Horman
2023-01-11 18:10 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).