[PATCH] netfiler: ipset: fix unaligned atomic access

[PATCH] netfiler: ipset: fix unaligned atomic access

[Russell King]

When using ip_set with counters and comment, traffic causes the kernel
to panic on 32-bit ARM:

Alignment trap: not handling instruction e1b82f9f at [<bf01b0dc>]
Unhandled fault: alignment exception (0x221) at 0xea08133c
PC is at ip_set_match_extensions+0xe0/0x224 [ip_set]

The problem occurs when we try to update the 64-bit counters - the
faulting address above is not 64-bit aligned.  The problem occurs
due to the way elements are allocated, for example:

	set->dsize = ip_set_elem_len(set, tb, 0, 0);
	map = ip_set_alloc(sizeof(*map) + elements * set->dsize);

If the element has a requirement for a member to be 64-bit aligned,
and set->dsize is not a multiple of 8, but is a multiple of four,
then every odd numbered elements will be misaligned - and hitting
an atomic64_add() on that element will cause the kernel to panic.

ip_set_elem_len() must return a size that is rounded to the maximum
alignment of any extension field stored in the element.  This change
ensures that is the case.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
Patch against v5.6, where I tripped over this bug.  This bug has
caused a kernel panic on my new router twice today.

 net/netfilter/ipset/ip_set_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 8dd17589217d..be9cd6a500fb 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -459,6 +459,8 @@ ip_set_elem_len(struct ip_set *set, struct nlattr *tb[], size_t len,
 	for (id = 0; id < IPSET_EXT_ID_MAX; id++) {
 		if (!add_extension(id, cadt_flags, tb))
 			continue;
+		if (align < ip_set_extensions[id].align)
+			align = ip_set_extensions[id].align;
 		len = ALIGN(len, ip_set_extensions[id].align);
 		set->offset[id] = len;
 		set->extensions |= ip_set_extensions[id].type;
-- 
2.20.1

Re: [PATCH] netfiler: ipset: fix unaligned atomic access

[Russell King - ARM Linux admin]

Gentle ping...

This patch fixes a remotely triggerable kernel oops, and as such can
be viewed as a remote denial of service attack depending on the
netfilter rule setup.

On Wed, Jun 10, 2020 at 09:51:11PM +0100, Russell King wrote:
> When using ip_set with counters and comment, traffic causes the kernel
> to panic on 32-bit ARM:
> 
> Alignment trap: not handling instruction e1b82f9f at [<bf01b0dc>]
> Unhandled fault: alignment exception (0x221) at 0xea08133c
> PC is at ip_set_match_extensions+0xe0/0x224 [ip_set]
> 
> The problem occurs when we try to update the 64-bit counters - the
> faulting address above is not 64-bit aligned.  The problem occurs
> due to the way elements are allocated, for example:
> 
> 	set->dsize = ip_set_elem_len(set, tb, 0, 0);
> 	map = ip_set_alloc(sizeof(*map) + elements * set->dsize);
> 
> If the element has a requirement for a member to be 64-bit aligned,
> and set->dsize is not a multiple of 8, but is a multiple of four,
> then every odd numbered elements will be misaligned - and hitting
> an atomic64_add() on that element will cause the kernel to panic.
> 
> ip_set_elem_len() must return a size that is rounded to the maximum
> alignment of any extension field stored in the element.  This change
> ensures that is the case.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> Patch against v5.6, where I tripped over this bug.  This bug has
> caused a kernel panic on my new router twice today.
> 
>  net/netfilter/ipset/ip_set_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> index 8dd17589217d..be9cd6a500fb 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -459,6 +459,8 @@ ip_set_elem_len(struct ip_set *set, struct nlattr *tb[], size_t len,
>  	for (id = 0; id < IPSET_EXT_ID_MAX; id++) {
>  		if (!add_extension(id, cadt_flags, tb))
>  			continue;
> +		if (align < ip_set_extensions[id].align)
> +			align = ip_set_extensions[id].align;
>  		len = ALIGN(len, ip_set_extensions[id].align);
>  		set->offset[id] = len;
>  		set->extensions |= ip_set_extensions[id].type;
> -- 
> 2.20.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] netfiler: ipset: fix unaligned atomic access

[Pablo Neira Ayuso]

Hi Jozsef,

I'll place in this in nf.git unless you have any objection.

Thanks.

On Sun, Jun 21, 2020 at 08:45:14PM +0100, Russell King - ARM Linux admin wrote:
> Gentle ping...
> 
> This patch fixes a remotely triggerable kernel oops, and as such can
> be viewed as a remote denial of service attack depending on the
> netfilter rule setup.
> 
> On Wed, Jun 10, 2020 at 09:51:11PM +0100, Russell King wrote:
> > When using ip_set with counters and comment, traffic causes the kernel
> > to panic on 32-bit ARM:
> > 
> > Alignment trap: not handling instruction e1b82f9f at [<bf01b0dc>]
> > Unhandled fault: alignment exception (0x221) at 0xea08133c
> > PC is at ip_set_match_extensions+0xe0/0x224 [ip_set]
> > 
> > The problem occurs when we try to update the 64-bit counters - the
> > faulting address above is not 64-bit aligned.  The problem occurs
> > due to the way elements are allocated, for example:
> > 
> > 	set->dsize = ip_set_elem_len(set, tb, 0, 0);
> > 	map = ip_set_alloc(sizeof(*map) + elements * set->dsize);
> > 
> > If the element has a requirement for a member to be 64-bit aligned,
> > and set->dsize is not a multiple of 8, but is a multiple of four,
> > then every odd numbered elements will be misaligned - and hitting
> > an atomic64_add() on that element will cause the kernel to panic.
> > 
> > ip_set_elem_len() must return a size that is rounded to the maximum
> > alignment of any extension field stored in the element.  This change
> > ensures that is the case.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> > Patch against v5.6, where I tripped over this bug.  This bug has
> > caused a kernel panic on my new router twice today.
> > 
> >  net/netfilter/ipset/ip_set_core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> > index 8dd17589217d..be9cd6a500fb 100644
> > --- a/net/netfilter/ipset/ip_set_core.c
> > +++ b/net/netfilter/ipset/ip_set_core.c
> > @@ -459,6 +459,8 @@ ip_set_elem_len(struct ip_set *set, struct nlattr *tb[], size_t len,
> >  	for (id = 0; id < IPSET_EXT_ID_MAX; id++) {
> >  		if (!add_extension(id, cadt_flags, tb))
> >  			continue;
> > +		if (align < ip_set_extensions[id].align)
> > +			align = ip_set_extensions[id].align;
> >  		len = ALIGN(len, ip_set_extensions[id].align);
> >  		set->offset[id] = len;
> >  		set->extensions |= ip_set_extensions[id].type;
> > -- 
> > 2.20.1
> > 
> > 
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] netfiler: ipset: fix unaligned atomic access

[Jozsef Kadlecsik]

Hi,

On Sun, 21 Jun 2020, Pablo Neira Ayuso wrote:

> I'll place in this in nf.git unless you have any objection.

No objection at all and thanks!

Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>

Best regards,
Jozsef

> On Sun, Jun 21, 2020 at 08:45:14PM +0100, Russell King - ARM Linux admin wrote:
> > Gentle ping...
> > 
> > This patch fixes a remotely triggerable kernel oops, and as such can
> > be viewed as a remote denial of service attack depending on the
> > netfilter rule setup.
> > 
> > On Wed, Jun 10, 2020 at 09:51:11PM +0100, Russell King wrote:
> > > When using ip_set with counters and comment, traffic causes the kernel
> > > to panic on 32-bit ARM:
> > > 
> > > Alignment trap: not handling instruction e1b82f9f at [<bf01b0dc>]
> > > Unhandled fault: alignment exception (0x221) at 0xea08133c
> > > PC is at ip_set_match_extensions+0xe0/0x224 [ip_set]
> > > 
> > > The problem occurs when we try to update the 64-bit counters - the
> > > faulting address above is not 64-bit aligned.  The problem occurs
> > > due to the way elements are allocated, for example:
> > > 
> > > 	set->dsize = ip_set_elem_len(set, tb, 0, 0);
> > > 	map = ip_set_alloc(sizeof(*map) + elements * set->dsize);
> > > 
> > > If the element has a requirement for a member to be 64-bit aligned,
> > > and set->dsize is not a multiple of 8, but is a multiple of four,
> > > then every odd numbered elements will be misaligned - and hitting
> > > an atomic64_add() on that element will cause the kernel to panic.
> > > 
> > > ip_set_elem_len() must return a size that is rounded to the maximum
> > > alignment of any extension field stored in the element.  This change
> > > ensures that is the case.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > ---
> > > Patch against v5.6, where I tripped over this bug.  This bug has
> > > caused a kernel panic on my new router twice today.
> > > 
> > >  net/netfilter/ipset/ip_set_core.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> > > index 8dd17589217d..be9cd6a500fb 100644
> > > --- a/net/netfilter/ipset/ip_set_core.c
> > > +++ b/net/netfilter/ipset/ip_set_core.c
> > > @@ -459,6 +459,8 @@ ip_set_elem_len(struct ip_set *set, struct nlattr *tb[], size_t len,
> > >  	for (id = 0; id < IPSET_EXT_ID_MAX; id++) {
> > >  		if (!add_extension(id, cadt_flags, tb))
> > >  			continue;
> > > +		if (align < ip_set_extensions[id].align)
> > > +			align = ip_set_extensions[id].align;
> > >  		len = ALIGN(len, ip_set_extensions[id].align);
> > >  		set->offset[id] = len;
> > >  		set->extensions |= ip_set_extensions[id].type;
> > > -- 
> > > 2.20.1
> > > 
> > > 
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.hu
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] netfiler: ipset: fix unaligned atomic access

[Pablo Neira Ayuso]

On Wed, Jun 10, 2020 at 09:51:11PM +0100, Russell King wrote:
> When using ip_set with counters and comment, traffic causes the kernel
> to panic on 32-bit ARM:
> 
> Alignment trap: not handling instruction e1b82f9f at [<bf01b0dc>]
> Unhandled fault: alignment exception (0x221) at 0xea08133c
> PC is at ip_set_match_extensions+0xe0/0x224 [ip_set]
> 
> The problem occurs when we try to update the 64-bit counters - the
> faulting address above is not 64-bit aligned.  The problem occurs
> due to the way elements are allocated, for example:
> 
> 	set->dsize = ip_set_elem_len(set, tb, 0, 0);
> 	map = ip_set_alloc(sizeof(*map) + elements * set->dsize);
> 
> If the element has a requirement for a member to be 64-bit aligned,
> and set->dsize is not a multiple of 8, but is a multiple of four,
> then every odd numbered elements will be misaligned - and hitting
> an atomic64_add() on that element will cause the kernel to panic.
> 
> ip_set_elem_len() must return a size that is rounded to the maximum
> alignment of any extension field stored in the element.  This change
> ensures that is the case.

Applied, thanks.

Re: [PATCH] netfiler: ipset: fix unaligned atomic access

[Russell King - ARM Linux admin]

On Mon, Jun 22, 2020 at 10:26:33AM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jun 10, 2020 at 09:51:11PM +0100, Russell King wrote:
> > When using ip_set with counters and comment, traffic causes the kernel
> > to panic on 32-bit ARM:
> > 
> > Alignment trap: not handling instruction e1b82f9f at [<bf01b0dc>]
> > Unhandled fault: alignment exception (0x221) at 0xea08133c
> > PC is at ip_set_match_extensions+0xe0/0x224 [ip_set]
> > 
> > The problem occurs when we try to update the 64-bit counters - the
> > faulting address above is not 64-bit aligned.  The problem occurs
> > due to the way elements are allocated, for example:
> > 
> > 	set->dsize = ip_set_elem_len(set, tb, 0, 0);
> > 	map = ip_set_alloc(sizeof(*map) + elements * set->dsize);
> > 
> > If the element has a requirement for a member to be 64-bit aligned,
> > and set->dsize is not a multiple of 8, but is a multiple of four,
> > then every odd numbered elements will be misaligned - and hitting
> > an atomic64_add() on that element will cause the kernel to panic.
> > 
> > ip_set_elem_len() must return a size that is rounded to the maximum
> > alignment of any extension field stored in the element.  This change
> > ensures that is the case.
> 
> Applied, thanks.

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!