* [PATCH 0/1] Netfilter OOB memory access security patch
@ 2020-07-27 17:57 Will McVicker
2020-07-27 17:57 ` [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] Will McVicker
0 siblings, 1 reply; 15+ messages in thread
From: Will McVicker @ 2020-07-27 17:57 UTC (permalink / raw)
To: security, Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
netfilter-devel, coreteam, netdev, linux-kernel, kernel-team,
Will McVicker
Hi,
The attached patch fixes an OOB memory access security bug. The bug is
already fixed in the upstream kernel due to the vulnerable code being
refactored in commit fe2d0020994c ("netfilter: nat: remove
l4proto->in_range") and commit d6c4c8ffb5e5 ("netfilter: nat: remove
l3proto struct"), but the 4.19 and below LTS branches remain vulnerable.
I have verifed the OOB kernel panic is fixed with this patch on both the
4.19 and 4.14 kernels using the approariate hardware.
Please review the fix and apply to branches 4.19.y, 4.14.y, 4.9.y and
4.4.y.
Thanks,
Will
Will McVicker (1):
netfilter: nat: add range checks for access to nf_nat_l[34]protos[]
net/ipv4/netfilter/nf_nat_l3proto_ipv4.c | 6 ++++--
net/ipv6/netfilter/nf_nat_l3proto_ipv6.c | 5 +++--
net/netfilter/nf_nat_core.c | 27 ++++++++++++++++++++++--
net/netfilter/nf_nat_helper.c | 4 ++++
4 files changed, 36 insertions(+), 6 deletions(-)
--
2.28.0.rc0.142.g3c755180ce-goog
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] 2020-07-27 17:57 [PATCH 0/1] Netfilter OOB memory access security patch Will McVicker @ 2020-07-27 17:57 ` Will McVicker 2020-07-29 21:46 ` Pablo Neira Ayuso 0 siblings, 1 reply; 15+ messages in thread From: Will McVicker @ 2020-07-27 17:57 UTC (permalink / raw) To: security, Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team, Will McVicker The indexes to the nf_nat_l[34]protos arrays come from userspace. So we need to make sure that before indexing the arrays, we verify the index is within the array bounds in order to prevent an OOB memory access. Here is an example kernel panic on 4.14.180 when userspace passes in an index greater than NFPROTO_NUMPROTO. Internal error: Oops - BUG: 0 [#1] PREEMPT SMP Modules linked in:... Process poc (pid: 5614, stack limit = 0x00000000a3933121) CPU: 4 PID: 5614 Comm: poc Tainted: G S W O 4.14.180-g051355490483 Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM task: 000000002a3dfffe task.stack: 00000000a3933121 pc : __cfi_check_fail+0x1c/0x24 lr : __cfi_check_fail+0x1c/0x24 ... Call trace: __cfi_check_fail+0x1c/0x24 name_to_dev_t+0x0/0x468 nfnetlink_parse_nat_setup+0x234/0x258 ctnetlink_parse_nat_setup+0x4c/0x228 ctnetlink_new_conntrack+0x590/0xc40 nfnetlink_rcv_msg+0x31c/0x4d4 netlink_rcv_skb+0x100/0x184 nfnetlink_rcv+0xf4/0x180 netlink_unicast+0x360/0x770 netlink_sendmsg+0x5a0/0x6a4 ___sys_sendmsg+0x314/0x46c SyS_sendmsg+0xb4/0x108 el0_svc_naked+0x34/0x38 Signed-off-by: Will McVicker <willmcvicker@google.com> --- net/ipv4/netfilter/nf_nat_l3proto_ipv4.c | 6 ++++-- net/ipv6/netfilter/nf_nat_l3proto_ipv6.c | 5 +++-- net/netfilter/nf_nat_core.c | 27 ++++++++++++++++++++++-- net/netfilter/nf_nat_helper.c | 4 ++++ 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c index 6115bf1ff6f0..1912fc66147c 100644 --- a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c +++ b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c @@ -218,7 +218,8 @@ int nf_nat_icmp_reply_translation(struct sk_buff *skb, return 1; l4proto = __nf_nat_l4proto_find(NFPROTO_IPV4, inside->ip.protocol); - if (!nf_nat_ipv4_manip_pkt(skb, hdrlen + sizeof(inside->icmp), + if (!l4proto || + !nf_nat_ipv4_manip_pkt(skb, hdrlen + sizeof(inside->icmp), l4proto, &ct->tuplehash[!dir].tuple, !manip)) return 0; @@ -234,7 +235,8 @@ int nf_nat_icmp_reply_translation(struct sk_buff *skb, /* Change outer to look like the reply to an incoming packet */ nf_ct_invert_tuplepr(&target, &ct->tuplehash[!dir].tuple); l4proto = __nf_nat_l4proto_find(NFPROTO_IPV4, 0); - if (!nf_nat_ipv4_manip_pkt(skb, 0, l4proto, &target, manip)) + if (!l4proto || + !nf_nat_ipv4_manip_pkt(skb, 0, l4proto, &target, manip)) return 0; return 1; diff --git a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c index ca6d38698b1a..a72840baf27b 100644 --- a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c +++ b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c @@ -228,7 +228,8 @@ int nf_nat_icmpv6_reply_translation(struct sk_buff *skb, return 1; l4proto = __nf_nat_l4proto_find(NFPROTO_IPV6, inside->ip6.nexthdr); - if (!nf_nat_ipv6_manip_pkt(skb, hdrlen + sizeof(inside->icmp6), + if (!l4proto || + !nf_nat_ipv6_manip_pkt(skb, hdrlen + sizeof(inside->icmp6), l4proto, &ct->tuplehash[!dir].tuple, !manip)) return 0; @@ -245,7 +246,7 @@ int nf_nat_icmpv6_reply_translation(struct sk_buff *skb, nf_ct_invert_tuplepr(&target, &ct->tuplehash[!dir].tuple); l4proto = __nf_nat_l4proto_find(NFPROTO_IPV6, IPPROTO_ICMPV6); - if (!nf_nat_ipv6_manip_pkt(skb, 0, l4proto, &target, manip)) + if (!l4proto || !nf_nat_ipv6_manip_pkt(skb, 0, l4proto, &target, manip)) return 0; return 1; diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c index 2268b10a9dcf..d28185f38955 100644 --- a/net/netfilter/nf_nat_core.c +++ b/net/netfilter/nf_nat_core.c @@ -64,12 +64,16 @@ struct nat_net { inline const struct nf_nat_l3proto * __nf_nat_l3proto_find(u8 family) { + if (family >= NFPROTO_NUMPROTO) + return NULL; return rcu_dereference(nf_nat_l3protos[family]); } inline const struct nf_nat_l4proto * __nf_nat_l4proto_find(u8 family, u8 protonum) { + if (family >= NFPROTO_NUMPROTO) + return NULL; return rcu_dereference(nf_nat_l4protos[family][protonum]); } EXPORT_SYMBOL_GPL(__nf_nat_l4proto_find); @@ -317,7 +321,7 @@ find_best_ips_proto(const struct nf_conntrack_zone *zone, * range. It might not be possible to get a unique tuple, but we try. * At worst (or if we race), we will end up with a final duplicate in * __ip_conntrack_confirm and drop the packet. */ -static void +static int get_unique_tuple(struct nf_conntrack_tuple *tuple, const struct nf_conntrack_tuple *orig_tuple, const struct nf_nat_range2 *range, @@ -328,13 +332,22 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple, const struct nf_nat_l3proto *l3proto; const struct nf_nat_l4proto *l4proto; struct net *net = nf_ct_net(ct); + int ret = 0; zone = nf_ct_zone(ct); rcu_read_lock(); l3proto = __nf_nat_l3proto_find(orig_tuple->src.l3num); + if (!l3proto) { + ret = -EINVAL; + goto out; + } l4proto = __nf_nat_l4proto_find(orig_tuple->src.l3num, orig_tuple->dst.protonum); + if (!l4proto) { + ret = -EINVAL; + goto out; + } /* 1) If this srcip/proto/src-proto-part is currently mapped, * and that same mapping gives a unique tuple within the given @@ -387,6 +400,7 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple, l4proto->unique_tuple(l3proto, tuple, range, maniptype, ct); out: rcu_read_unlock(); + return ret; } struct nf_conn_nat *nf_ct_nat_ext_add(struct nf_conn *ct) @@ -409,6 +423,7 @@ nf_nat_setup_info(struct nf_conn *ct, { struct net *net = nf_ct_net(ct); struct nf_conntrack_tuple curr_tuple, new_tuple; + int err; /* Can't setup nat info for confirmed ct. */ if (nf_ct_is_confirmed(ct)) @@ -428,7 +443,9 @@ nf_nat_setup_info(struct nf_conn *ct, nf_ct_invert_tuplepr(&curr_tuple, &ct->tuplehash[IP_CT_DIR_REPLY].tuple); - get_unique_tuple(&new_tuple, &curr_tuple, range, ct, maniptype); + err = get_unique_tuple(&new_tuple, &curr_tuple, range, ct, maniptype); + if (err < 0) + return NF_DROP; if (!nf_ct_tuple_equal(&new_tuple, &curr_tuple)) { struct nf_conntrack_tuple reply; @@ -509,8 +526,12 @@ static unsigned int nf_nat_manip_pkt(struct sk_buff *skb, struct nf_conn *ct, nf_ct_invert_tuplepr(&target, &ct->tuplehash[!dir].tuple); l3proto = __nf_nat_l3proto_find(target.src.l3num); + if (!l3proto) + return NF_DROP; l4proto = __nf_nat_l4proto_find(target.src.l3num, target.dst.protonum); + if (!l4proto) + return NF_DROP; if (!l3proto->manip_pkt(skb, 0, l4proto, &target, mtype)) return NF_DROP; @@ -816,6 +837,8 @@ static int nfnetlink_parse_nat_proto(struct nlattr *attr, return err; l4proto = __nf_nat_l4proto_find(nf_ct_l3num(ct), nf_ct_protonum(ct)); + if (!l4proto) + return -EINVAL; if (l4proto->nlattr_to_range) err = l4proto->nlattr_to_range(tb, range); diff --git a/net/netfilter/nf_nat_helper.c b/net/netfilter/nf_nat_helper.c index 99606baedda4..6f694444137e 100644 --- a/net/netfilter/nf_nat_helper.c +++ b/net/netfilter/nf_nat_helper.c @@ -121,6 +121,8 @@ bool __nf_nat_mangle_tcp_packet(struct sk_buff *skb, datalen = skb->len - protoff; l3proto = __nf_nat_l3proto_find(nf_ct_l3num(ct)); + if (!l3proto) + return false; l3proto->csum_recalc(skb, IPPROTO_TCP, tcph, &tcph->check, datalen, oldlen); @@ -179,6 +181,8 @@ nf_nat_mangle_udp_packet(struct sk_buff *skb, return true; l3proto = __nf_nat_l3proto_find(nf_ct_l3num(ct)); + if (!l3proto) + return false; l3proto->csum_recalc(skb, IPPROTO_UDP, udph, &udph->check, datalen, oldlen); -- 2.28.0.rc0.142.g3c755180ce-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] 2020-07-27 17:57 ` [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] Will McVicker @ 2020-07-29 21:46 ` Pablo Neira Ayuso 2020-07-31 0:26 ` William Mcvicker 0 siblings, 1 reply; 15+ messages in thread From: Pablo Neira Ayuso @ 2020-07-29 21:46 UTC (permalink / raw) To: Will McVicker Cc: security, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team Hi Will, On Mon, Jul 27, 2020 at 05:57:20PM +0000, Will McVicker wrote: > The indexes to the nf_nat_l[34]protos arrays come from userspace. So we > need to make sure that before indexing the arrays, we verify the index > is within the array bounds in order to prevent an OOB memory access. > Here is an example kernel panic on 4.14.180 when userspace passes in an > index greater than NFPROTO_NUMPROTO. > > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP > Modules linked in:... > Process poc (pid: 5614, stack limit = 0x00000000a3933121) > CPU: 4 PID: 5614 Comm: poc Tainted: G S W O 4.14.180-g051355490483 > Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM > task: 000000002a3dfffe task.stack: 00000000a3933121 > pc : __cfi_check_fail+0x1c/0x24 > lr : __cfi_check_fail+0x1c/0x24 > ... > Call trace: > __cfi_check_fail+0x1c/0x24 > name_to_dev_t+0x0/0x468 > nfnetlink_parse_nat_setup+0x234/0x258 If this oops is only triggerable from userspace, I think a sanity check in nfnetlink_parse_nat_setup should suffice to reject unsupported layer 3 and layer 4 protocols. I mean, in this patch I see more chunks in the packet path, such as nf_nat_l3proto_ipv4 that should never happen. I would just fix the userspace ctnetlink path. BTW, do you have a Fixes: tag for this? This will be useful for -stable maintainer to pick up this fix. Thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] 2020-07-29 21:46 ` Pablo Neira Ayuso @ 2020-07-31 0:26 ` William Mcvicker 2020-07-31 17:51 ` Pablo Neira Ayuso 0 siblings, 1 reply; 15+ messages in thread From: William Mcvicker @ 2020-07-31 0:26 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: security, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team Hi Pablo, Yes, I believe this oops is only triggered by userspace when the user specifically passes in an invalid nf_nat_l3protos index. I'm happy to re-work the patch to check for this in ctnetlink_create_conntrack(). > BTW, do you have a Fixes: tag for this? This will be useful for > -stable maintainer to pick up this fix. Regarding the Fixes: tag, I don't have one offhand since this bug was reported to me, but I can search through the code history to find the commit that exposed this vulnerability. Thanks, Will On 07/29/2020, Pablo Neira Ayuso wrote: > Hi Will, > > On Mon, Jul 27, 2020 at 05:57:20PM +0000, Will McVicker wrote: > > The indexes to the nf_nat_l[34]protos arrays come from userspace. So we > > need to make sure that before indexing the arrays, we verify the index > > is within the array bounds in order to prevent an OOB memory access. > > Here is an example kernel panic on 4.14.180 when userspace passes in an > > index greater than NFPROTO_NUMPROTO. > > > > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP > > Modules linked in:... > > Process poc (pid: 5614, stack limit = 0x00000000a3933121) > > CPU: 4 PID: 5614 Comm: poc Tainted: G S W O 4.14.180-g051355490483 > > Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM > > task: 000000002a3dfffe task.stack: 00000000a3933121 > > pc : __cfi_check_fail+0x1c/0x24 > > lr : __cfi_check_fail+0x1c/0x24 > > ... > > Call trace: > > __cfi_check_fail+0x1c/0x24 > > name_to_dev_t+0x0/0x468 > > nfnetlink_parse_nat_setup+0x234/0x258 > > If this oops is only triggerable from userspace, I think a sanity > check in nfnetlink_parse_nat_setup should suffice to reject > unsupported layer 3 and layer 4 protocols. > > I mean, in this patch I see more chunks in the packet path, such as > nf_nat_l3proto_ipv4 that should never happen. I would just fix the > userspace ctnetlink path. > > BTW, do you have a Fixes: tag for this? This will be useful for > -stable maintainer to pick up this fix. > > Thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] 2020-07-31 0:26 ` William Mcvicker @ 2020-07-31 17:51 ` Pablo Neira Ayuso 2020-07-31 18:16 ` William Mcvicker 0 siblings, 1 reply; 15+ messages in thread From: Pablo Neira Ayuso @ 2020-07-31 17:51 UTC (permalink / raw) To: William Mcvicker Cc: security, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team Hi William, On Fri, Jul 31, 2020 at 12:26:11AM +0000, William Mcvicker wrote: > Hi Pablo, > > Yes, I believe this oops is only triggered by userspace when the user > specifically passes in an invalid nf_nat_l3protos index. I'm happy to re-work > the patch to check for this in ctnetlink_create_conntrack(). Great. Note that this code does not exist in the tree anymore. I'm not sure if this problem still exists upstream, this patch does not apply to nf.git. This fix should only go for -stable maintainers. > > BTW, do you have a Fixes: tag for this? This will be useful for > > -stable maintainer to pick up this fix. > > Regarding the Fixes: tag, I don't have one offhand since this bug was reported > to me, but I can search through the code history to find the commit that > exposed this vulnerability. That would be great. Thank you. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] 2020-07-31 17:51 ` Pablo Neira Ayuso @ 2020-07-31 18:16 ` William Mcvicker 2020-08-03 18:31 ` [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum William Mcvicker 0 siblings, 1 reply; 15+ messages in thread From: William Mcvicker @ 2020-07-31 18:16 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: security, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team Hi Pablo, > Note that this code does not exist in the tree anymore. I'm not sure > if this problem still exists upstream, this patch does not apply to > nf.git. This fix should only go for -stable maintainers. Right, the vulnerability has been fixed by the refactor commit fe2d0020994cd ("netfilter: nat: remove l4proto->in_range"), but this patch is a part of a full re-work of the code and doesn't backport very cleanly to the LTS branches. So this fix is only applicable to the 4.19, 4.14, 4.9, and 4.4 LTS branches. I missed the -stable email, but will re-add it to this thread with the re-worked patch. Thanks, Will On 07/31/2020, Pablo Neira Ayuso wrote: > Hi William, > > On Fri, Jul 31, 2020 at 12:26:11AM +0000, William Mcvicker wrote: > > Hi Pablo, > > > > Yes, I believe this oops is only triggered by userspace when the user > > specifically passes in an invalid nf_nat_l3protos index. I'm happy to re-work > > the patch to check for this in ctnetlink_create_conntrack(). > > Great. > > Note that this code does not exist in the tree anymore. I'm not sure > if this problem still exists upstream, this patch does not apply to > nf.git. This fix should only go for -stable maintainers. > > > > BTW, do you have a Fixes: tag for this? This will be useful for > > > -stable maintainer to pick up this fix. > > > > Regarding the Fixes: tag, I don't have one offhand since this bug was reported > > to me, but I can search through the code history to find the commit that > > exposed this vulnerability. > > That would be great. > > Thank you. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum 2020-07-31 18:16 ` William Mcvicker @ 2020-08-03 18:31 ` William Mcvicker 2020-08-04 11:37 ` Pablo Neira Ayuso 0 siblings, 1 reply; 15+ messages in thread From: William Mcvicker @ 2020-08-03 18:31 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: security, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team, stable [-- Attachment #1: Type: text/plain, Size: 1799 bytes --] Hi, I have attached the follow up fix that checks for the proto index during conntrack creation. Thanks, Will On 07/31/2020, William Mcvicker wrote: > Hi Pablo, > > > Note that this code does not exist in the tree anymore. I'm not sure > > if this problem still exists upstream, this patch does not apply to > > nf.git. This fix should only go for -stable maintainers. > > Right, the vulnerability has been fixed by the refactor commit fe2d0020994cd > ("netfilter: nat: remove l4proto->in_range"), but this patch is a part of > a full re-work of the code and doesn't backport very cleanly to the LTS > branches. So this fix is only applicable to the 4.19, 4.14, 4.9, and 4.4 LTS > branches. I missed the -stable email, but will re-add it to this thread with > the re-worked patch. > > Thanks, > Will > > On 07/31/2020, Pablo Neira Ayuso wrote: > > Hi William, > > > > On Fri, Jul 31, 2020 at 12:26:11AM +0000, William Mcvicker wrote: > > > Hi Pablo, > > > > > > Yes, I believe this oops is only triggered by userspace when the user > > > specifically passes in an invalid nf_nat_l3protos index. I'm happy to re-work > > > the patch to check for this in ctnetlink_create_conntrack(). > > > > Great. > > > > Note that this code does not exist in the tree anymore. I'm not sure > > if this problem still exists upstream, this patch does not apply to > > nf.git. This fix should only go for -stable maintainers. > > > > > > BTW, do you have a Fixes: tag for this? This will be useful for > > > > -stable maintainer to pick up this fix. > > > > > > Regarding the Fixes: tag, I don't have one offhand since this bug was reported > > > to me, but I can search through the code history to find the commit that > > > exposed this vulnerability. > > > > That would be great. > > > > Thank you. [-- Attachment #2: 0001-netfilter-nat-add-a-range-check-for-l3-l4-protonum.patch --] [-- Type: text/x-diff, Size: 2041 bytes --] From 2a9d621fa5c225e6aece6b4622a9a816c6fcfa0d Mon Sep 17 00:00:00 2001 From: Will McVicker <willmcvicker@google.com> Date: Fri, 31 Jul 2020 13:10:43 -0700 Subject: [PATCH] netfilter: nat: add a range check for l3/l4 protonum The indexes to the nf_nat_l[34]protos arrays come from userspace. So check the tuple's family, e.g. l3num, when creating the conntrack in order to prevent an OOB memory access during setup. Here is an example kernel panic on 4.14.180 when userspace passes in an index greater than NFPROTO_NUMPROTO. Internal error: Oops - BUG: 0 [#1] PREEMPT SMP Modules linked in:... Process poc (pid: 5614, stack limit = 0x00000000a3933121) CPU: 4 PID: 5614 Comm: poc Tainted: G S W O 4.14.180-g051355490483 Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM task: 000000002a3dfffe task.stack: 00000000a3933121 pc : __cfi_check_fail+0x1c/0x24 lr : __cfi_check_fail+0x1c/0x24 ... Call trace: __cfi_check_fail+0x1c/0x24 name_to_dev_t+0x0/0x468 nfnetlink_parse_nat_setup+0x234/0x258 ctnetlink_parse_nat_setup+0x4c/0x228 ctnetlink_new_conntrack+0x590/0xc40 nfnetlink_rcv_msg+0x31c/0x4d4 netlink_rcv_skb+0x100/0x184 nfnetlink_rcv+0xf4/0x180 netlink_unicast+0x360/0x770 netlink_sendmsg+0x5a0/0x6a4 ___sys_sendmsg+0x314/0x46c SyS_sendmsg+0xb4/0x108 el0_svc_naked+0x34/0x38 Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack") Signed-off-by: Will McVicker <willmcvicker@google.com> --- net/netfilter/nf_conntrack_netlink.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index 31fa94064a62..56d310f8b29a 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[], if (!tb[CTA_TUPLE_IP]) return -EINVAL; + if (l3num >= NFPROTO_NUMPROTO) + return -EINVAL; tuple->src.l3num = l3num; err = ctnetlink_parse_tuple_ip(tb[CTA_TUPLE_IP], tuple); -- 2.28.0.163.g6104cc2f0b6-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum 2020-08-03 18:31 ` [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum William Mcvicker @ 2020-08-04 11:37 ` Pablo Neira Ayuso 2020-08-24 19:38 ` [PATCH v3 0/1] " Will McVicker 2020-09-01 15:36 ` [PATCH v2 " Will Deacon 0 siblings, 2 replies; 15+ messages in thread From: Pablo Neira Ayuso @ 2020-08-04 11:37 UTC (permalink / raw) To: William Mcvicker Cc: security, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team, stable Hi, This patch is much smaller and if you confirm this is address the issue, then this is awesome. On Mon, Aug 03, 2020 at 06:31:56PM +0000, William Mcvicker wrote: [...] > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c > index 31fa94064a62..56d310f8b29a 100644 > --- a/net/netfilter/nf_conntrack_netlink.c > +++ b/net/netfilter/nf_conntrack_netlink.c > @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[], > if (!tb[CTA_TUPLE_IP]) > return -EINVAL; > > + if (l3num >= NFPROTO_NUMPROTO) > + return -EINVAL; l3num can only be either NFPROTO_IPV4 or NFPROTO_IPV6. Other than that, bail out with EOPNOTSUPP. Thank you. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 0/1] netfilter: nat: add a range check for l3/l4 protonum 2020-08-04 11:37 ` Pablo Neira Ayuso @ 2020-08-24 19:38 ` Will McVicker 2020-08-24 19:38 ` [PATCH v3 1/1] " Will McVicker 2020-09-01 15:36 ` [PATCH v2 " Will Deacon 1 sibling, 1 reply; 15+ messages in thread From: Will McVicker @ 2020-08-24 19:38 UTC (permalink / raw) To: stable, Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team, Will McVicker Hi Pablo, > This patch is much smaller and if you confirm this is address the > issue, then this is awesome. Yes, I can confirm the updated patch does fix the kernel panic. I have retested on the Pixel 4 XL with version 4.14.180. Please see the updated patchset v3. Thanks, Will Will McVicker (1): netfilter: nat: add a range check for l3/l4 protonum net/netfilter/nf_conntrack_netlink.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.28.0.297.g1956fa8f8d-goog ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/1] netfilter: nat: add a range check for l3/l4 protonum 2020-08-24 19:38 ` [PATCH v3 0/1] " Will McVicker @ 2020-08-24 19:38 ` Will McVicker 2020-08-28 16:42 ` Pablo Neira Ayuso 0 siblings, 1 reply; 15+ messages in thread From: Will McVicker @ 2020-08-24 19:38 UTC (permalink / raw) To: stable, Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team, Will McVicker The indexes to the nf_nat_l[34]protos arrays come from userspace. So check the tuple's family, e.g. l3num, when creating the conntrack in order to prevent an OOB memory access during setup. Here is an example kernel panic on 4.14.180 when userspace passes in an index greater than NFPROTO_NUMPROTO. Internal error: Oops - BUG: 0 [#1] PREEMPT SMP Modules linked in:... Process poc (pid: 5614, stack limit = 0x00000000a3933121) CPU: 4 PID: 5614 Comm: poc Tainted: G S W O 4.14.180-g051355490483 Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM task: 000000002a3dfffe task.stack: 00000000a3933121 pc : __cfi_check_fail+0x1c/0x24 lr : __cfi_check_fail+0x1c/0x24 ... Call trace: __cfi_check_fail+0x1c/0x24 name_to_dev_t+0x0/0x468 nfnetlink_parse_nat_setup+0x234/0x258 ctnetlink_parse_nat_setup+0x4c/0x228 ctnetlink_new_conntrack+0x590/0xc40 nfnetlink_rcv_msg+0x31c/0x4d4 netlink_rcv_skb+0x100/0x184 nfnetlink_rcv+0xf4/0x180 netlink_unicast+0x360/0x770 netlink_sendmsg+0x5a0/0x6a4 ___sys_sendmsg+0x314/0x46c SyS_sendmsg+0xb4/0x108 el0_svc_naked+0x34/0x38 Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack") Signed-off-by: Will McVicker <willmcvicker@google.com> --- net/netfilter/nf_conntrack_netlink.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index 31fa94064a62..0b89609a6e9d 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[], if (!tb[CTA_TUPLE_IP]) return -EINVAL; + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6) + return -EOPNOTSUPP; tuple->src.l3num = l3num; err = ctnetlink_parse_tuple_ip(tb[CTA_TUPLE_IP], tuple); -- 2.28.0.297.g1956fa8f8d-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/1] netfilter: nat: add a range check for l3/l4 protonum 2020-08-24 19:38 ` [PATCH v3 1/1] " Will McVicker @ 2020-08-28 16:42 ` Pablo Neira Ayuso 2020-08-28 16:45 ` Florian Westphal 0 siblings, 1 reply; 15+ messages in thread From: Pablo Neira Ayuso @ 2020-08-28 16:42 UTC (permalink / raw) To: Will McVicker Cc: stable, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team Hi Will, Given this is for -stable maintainers only, I'd suggest: 1) Specify what -stable kernel versions this patch applies to. Explain that this problem is gone since what kernel version. 2) Maybe clarify that this is only for stable in the patch subject, e.g. [PATCH -stable v3] netfilter: nat: add a range check for l3/l4 Otherwise, this -stable maintainers might not identify this patch as something that is targetted to them. Thanks. On Mon, Aug 24, 2020 at 07:38:32PM +0000, Will McVicker wrote: > The indexes to the nf_nat_l[34]protos arrays come from userspace. So > check the tuple's family, e.g. l3num, when creating the conntrack in > order to prevent an OOB memory access during setup. Here is an example > kernel panic on 4.14.180 when userspace passes in an index greater than > NFPROTO_NUMPROTO. > > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP > Modules linked in:... > Process poc (pid: 5614, stack limit = 0x00000000a3933121) > CPU: 4 PID: 5614 Comm: poc Tainted: G S W O 4.14.180-g051355490483 > Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM > task: 000000002a3dfffe task.stack: 00000000a3933121 > pc : __cfi_check_fail+0x1c/0x24 > lr : __cfi_check_fail+0x1c/0x24 > ... > Call trace: > __cfi_check_fail+0x1c/0x24 > name_to_dev_t+0x0/0x468 > nfnetlink_parse_nat_setup+0x234/0x258 > ctnetlink_parse_nat_setup+0x4c/0x228 > ctnetlink_new_conntrack+0x590/0xc40 > nfnetlink_rcv_msg+0x31c/0x4d4 > netlink_rcv_skb+0x100/0x184 > nfnetlink_rcv+0xf4/0x180 > netlink_unicast+0x360/0x770 > netlink_sendmsg+0x5a0/0x6a4 > ___sys_sendmsg+0x314/0x46c > SyS_sendmsg+0xb4/0x108 > el0_svc_naked+0x34/0x38 > > Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack") > Signed-off-by: Will McVicker <willmcvicker@google.com> > --- > net/netfilter/nf_conntrack_netlink.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c > index 31fa94064a62..0b89609a6e9d 100644 > --- a/net/netfilter/nf_conntrack_netlink.c > +++ b/net/netfilter/nf_conntrack_netlink.c > @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[], > if (!tb[CTA_TUPLE_IP]) > return -EINVAL; > > + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6) > + return -EOPNOTSUPP; > tuple->src.l3num = l3num; > > err = ctnetlink_parse_tuple_ip(tb[CTA_TUPLE_IP], tuple); > -- > 2.28.0.297.g1956fa8f8d-goog > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/1] netfilter: nat: add a range check for l3/l4 protonum 2020-08-28 16:42 ` Pablo Neira Ayuso @ 2020-08-28 16:45 ` Florian Westphal 2020-08-28 17:11 ` Pablo Neira Ayuso 0 siblings, 1 reply; 15+ messages in thread From: Florian Westphal @ 2020-08-28 16:45 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: Will McVicker, stable, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team Pablo Neira Ayuso <pablo@netfilter.org> wrote: > Hi Will, > > Given this is for -stable maintainers only, I'd suggest: > > 1) Specify what -stable kernel versions this patch applies to. > Explain that this problem is gone since what kernel version. > > 2) Maybe clarify that this is only for stable in the patch subject, > e.g. [PATCH -stable v3] netfilter: nat: add a range check for l3/l4 Hmm, we silently accept a tuple that we can't really deal with, no? > > + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6) > > + return -EOPNOTSUPP; I vote to apply this to nf.git ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/1] netfilter: nat: add a range check for l3/l4 protonum 2020-08-28 16:45 ` Florian Westphal @ 2020-08-28 17:11 ` Pablo Neira Ayuso 0 siblings, 0 replies; 15+ messages in thread From: Pablo Neira Ayuso @ 2020-08-28 17:11 UTC (permalink / raw) To: Florian Westphal Cc: Will McVicker, stable, Jozsef Kadlecsik, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team [-- Attachment #1: Type: text/plain, Size: 830 bytes --] On Fri, Aug 28, 2020 at 06:45:51PM +0200, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Hi Will, > > > > Given this is for -stable maintainers only, I'd suggest: > > > > 1) Specify what -stable kernel versions this patch applies to. > > Explain that this problem is gone since what kernel version. > > > > 2) Maybe clarify that this is only for stable in the patch subject, > > e.g. [PATCH -stable v3] netfilter: nat: add a range check for l3/l4 > > Hmm, we silently accept a tuple that we can't really deal with, no? Oh, I overlook, existing kernels are affected. You're right. > > > + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6) > > > + return -EOPNOTSUPP; > > I vote to apply this to nf.git I have rebased this patch on top of nf.git, attached what I'll apply to nf.git. [-- Attachment #2: 0001-netfilter-ctnetlink-add-a-range-check-for-l3-l4-prot.patch --] [-- Type: text/x-diff, Size: 2165 bytes --] From 4d3426b91eba6eb28f38a2b06ee024aff861aa16 Mon Sep 17 00:00:00 2001 From: Will McVicker <willmcvicker@google.com> Date: Mon, 24 Aug 2020 19:38:32 +0000 Subject: [PATCH] netfilter: ctnetlink: add a range check for l3/l4 protonum The indexes to the nf_nat_l[34]protos arrays come from userspace. So check the tuple's family, e.g. l3num, when creating the conntrack in order to prevent an OOB memory access during setup. Here is an example kernel panic on 4.14.180 when userspace passes in an index greater than NFPROTO_NUMPROTO. Internal error: Oops - BUG: 0 [#1] PREEMPT SMP Modules linked in:... Process poc (pid: 5614, stack limit = 0x00000000a3933121) CPU: 4 PID: 5614 Comm: poc Tainted: G S W O 4.14.180-g051355490483 Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM task: 000000002a3dfffe task.stack: 00000000a3933121 pc : __cfi_check_fail+0x1c/0x24 lr : __cfi_check_fail+0x1c/0x24 ... Call trace: __cfi_check_fail+0x1c/0x24 name_to_dev_t+0x0/0x468 nfnetlink_parse_nat_setup+0x234/0x258 ctnetlink_parse_nat_setup+0x4c/0x228 ctnetlink_new_conntrack+0x590/0xc40 nfnetlink_rcv_msg+0x31c/0x4d4 netlink_rcv_skb+0x100/0x184 nfnetlink_rcv+0xf4/0x180 netlink_unicast+0x360/0x770 netlink_sendmsg+0x5a0/0x6a4 ___sys_sendmsg+0x314/0x46c SyS_sendmsg+0xb4/0x108 el0_svc_naked+0x34/0x38 Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack") Signed-off-by: Will McVicker <willmcvicker@google.com> [pablo@netfilter.org: rebased original patch on top of nf.git] Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nf_conntrack_netlink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index 832eabecfbdd..d65846aa8059 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -1404,7 +1404,8 @@ ctnetlink_parse_tuple_filter(const struct nlattr * const cda[], if (err < 0) return err; - + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6) + return -EOPNOTSUPP; tuple->src.l3num = l3num; if (flags & CTA_FILTER_FLAG(CTA_IP_DST) || -- 2.20.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum 2020-08-04 11:37 ` Pablo Neira Ayuso 2020-08-24 19:38 ` [PATCH v3 0/1] " Will McVicker @ 2020-09-01 15:36 ` Will Deacon 2020-09-01 17:29 ` William Mcvicker 1 sibling, 1 reply; 15+ messages in thread From: Will Deacon @ 2020-09-01 15:36 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: William Mcvicker, security, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team, stable Hi Will, Pablo, On Tue, Aug 04, 2020 at 01:37:11PM +0200, Pablo Neira Ayuso wrote: > This patch is much smaller and if you confirm this is address the > issue, then this is awesome. Did that ever get confirmed? AFAICT, nothing ended up landing in the stable trees for this. Cheers, Will > On Mon, Aug 03, 2020 at 06:31:56PM +0000, William Mcvicker wrote: > [...] > > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c > > index 31fa94064a62..56d310f8b29a 100644 > > --- a/net/netfilter/nf_conntrack_netlink.c > > +++ b/net/netfilter/nf_conntrack_netlink.c > > @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[], > > if (!tb[CTA_TUPLE_IP]) > > return -EINVAL; > > > > + if (l3num >= NFPROTO_NUMPROTO) > > + return -EINVAL; > > l3num can only be either NFPROTO_IPV4 or NFPROTO_IPV6. > > Other than that, bail out with EOPNOTSUPP. > > Thank you. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum 2020-09-01 15:36 ` [PATCH v2 " Will Deacon @ 2020-09-01 17:29 ` William Mcvicker 0 siblings, 0 replies; 15+ messages in thread From: William Mcvicker @ 2020-09-01 17:29 UTC (permalink / raw) To: Will Deacon Cc: Pablo Neira Ayuso, security, Jozsef Kadlecsik, Florian Westphal, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev, linux-kernel, kernel-team, stable Hi Will, Pablo is going to add the latest patch to the nf.git tree. Once that happens, I'm going to propose the patch in nf.git get cherry-picked to the -stable branches. Thanks, Will On Tue, Sep 1, 2020 at 8:36 AM Will Deacon <will@kernel.org> wrote: > > Hi Will, Pablo, > > On Tue, Aug 04, 2020 at 01:37:11PM +0200, Pablo Neira Ayuso wrote: > > This patch is much smaller and if you confirm this is address the > > issue, then this is awesome. > > Did that ever get confirmed? AFAICT, nothing ended up landing in the stable > trees for this. > > Cheers, > > Will > > > > On Mon, Aug 03, 2020 at 06:31:56PM +0000, William Mcvicker wrote: > > [...] > > > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c > > > index 31fa94064a62..56d310f8b29a 100644 > > > --- a/net/netfilter/nf_conntrack_netlink.c > > > +++ b/net/netfilter/nf_conntrack_netlink.c > > > @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[], > > > if (!tb[CTA_TUPLE_IP]) > > > return -EINVAL; > > > > > > + if (l3num >= NFPROTO_NUMPROTO) > > > + return -EINVAL; > > > > l3num can only be either NFPROTO_IPV4 or NFPROTO_IPV6. > > > > Other than that, bail out with EOPNOTSUPP. > > > > Thank you. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-09-01 17:29 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-27 17:57 [PATCH 0/1] Netfilter OOB memory access security patch Will McVicker 2020-07-27 17:57 ` [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] Will McVicker 2020-07-29 21:46 ` Pablo Neira Ayuso 2020-07-31 0:26 ` William Mcvicker 2020-07-31 17:51 ` Pablo Neira Ayuso 2020-07-31 18:16 ` William Mcvicker 2020-08-03 18:31 ` [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum William Mcvicker 2020-08-04 11:37 ` Pablo Neira Ayuso 2020-08-24 19:38 ` [PATCH v3 0/1] " Will McVicker 2020-08-24 19:38 ` [PATCH v3 1/1] " Will McVicker 2020-08-28 16:42 ` Pablo Neira Ayuso 2020-08-28 16:45 ` Florian Westphal 2020-08-28 17:11 ` Pablo Neira Ayuso 2020-09-01 15:36 ` [PATCH v2 " Will Deacon 2020-09-01 17:29 ` William Mcvicker
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).