| Message ID | E1jj7gl-0008Bq-BQ@rmk-PC.armlinux.org.uk |
|---|---|
| State | Accepted |
| Delegated to: | Jozsef Kadlecsik |
| Headers | show |
| Series | netfiler: ipset: fix unaligned atomic access | expand |
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 > >
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!
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
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.
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!
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;
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(+)