From c7d13358b6a2f49f81a34aa323a2d0878a0532a2 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Fri, 30 Apr 2021 14:00:13 +0200 Subject: netfilter: xt_SECMARK: add new revision to fix structure layout This extension breaks when trying to delete rules, add a new revision to fix this. Fixes: 5e6874cdb8de ("[SECMARK]: Add xtables SECMARK target") Signed-off-by: Phil Sutter Signed-off-by: Pablo Neira Ayuso --- include/uapi/linux/netfilter/xt_SECMARK.h | 6 +++ net/netfilter/xt_SECMARK.c | 88 ++++++++++++++++++++++++------- 2 files changed, 75 insertions(+), 19 deletions(-) diff --git a/include/uapi/linux/netfilter/xt_SECMARK.h b/include/uapi/linux/netfilter/xt_SECMARK.h index 1f2a708413f5..beb2cadba8a9 100644 --- a/include/uapi/linux/netfilter/xt_SECMARK.h +++ b/include/uapi/linux/netfilter/xt_SECMARK.h @@ -20,4 +20,10 @@ struct xt_secmark_target_info { char secctx[SECMARK_SECCTX_MAX]; }; +struct xt_secmark_target_info_v1 { + __u8 mode; + char secctx[SECMARK_SECCTX_MAX]; + __u32 secid; +}; + #endif /*_XT_SECMARK_H_target */ diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c index 75625d13e976..498a0bf6f044 100644 --- a/net/netfilter/xt_SECMARK.c +++ b/net/netfilter/xt_SECMARK.c @@ -24,10 +24,9 @@ MODULE_ALIAS("ip6t_SECMARK"); static u8 mode; static unsigned int -secmark_tg(struct sk_buff *skb, const struct xt_action_param *par) +secmark_tg(struct sk_buff *skb, const struct xt_secmark_target_info_v1 *info) { u32 secmark = 0; - const struct xt_secmark_target_info *info = par->targinfo; switch (mode) { case SECMARK_MODE_SEL: @@ -41,7 +40,7 @@ secmark_tg(struct sk_buff *skb, const struct xt_action_param *par) return XT_CONTINUE; } -static int checkentry_lsm(struct xt_secmark_target_info *info) +static int checkentry_lsm(struct xt_secmark_target_info_v1 *info) { int err; @@ -73,15 +72,15 @@ static int checkentry_lsm(struct xt_secmark_target_info *info) return 0; } -static int secmark_tg_check(const struct xt_tgchk_param *par) +static int +secmark_tg_check(const char *table, struct xt_secmark_target_info_v1 *info) { - struct xt_secmark_target_info *info = par->targinfo; int err; - if (strcmp(par->table, "mangle") != 0 && - strcmp(par->table, "security") != 0) { + if (strcmp(table, "mangle") != 0 && + strcmp(table, "security") != 0) { pr_info_ratelimited("only valid in \'mangle\' or \'security\' table, not \'%s\'\n", - par->table); + table); return -EINVAL; } @@ -116,25 +115,76 @@ static void secmark_tg_destroy(const struct xt_tgdtor_param *par) } } -static struct xt_target secmark_tg_reg __read_mostly = { - .name = "SECMARK", - .revision = 0, - .family = NFPROTO_UNSPEC, - .checkentry = secmark_tg_check, - .destroy = secmark_tg_destroy, - .target = secmark_tg, - .targetsize = sizeof(struct xt_secmark_target_info), - .me = THIS_MODULE, +static int secmark_tg_check_v0(const struct xt_tgchk_param *par) +{ + struct xt_secmark_target_info *info = par->targinfo; + struct xt_secmark_target_info_v1 newinfo = { + .mode = info->mode, + }; + int ret; + + memcpy(newinfo.secctx, info->secctx, SECMARK_SECCTX_MAX); + + ret = secmark_tg_check(par->table, &newinfo); + info->secid = newinfo.secid; + + return ret; +} + +static unsigned int +secmark_tg_v0(struct sk_buff *skb, const struct xt_action_param *par) +{ + const struct xt_secmark_target_info *info = par->targinfo; + struct xt_secmark_target_info_v1 newinfo = { + .secid = info->secid, + }; + + return secmark_tg(skb, &newinfo); +} + +static int secmark_tg_check_v1(const struct xt_tgchk_param *par) +{ + return secmark_tg_check(par->table, par->targinfo); +} + +static unsigned int +secmark_tg_v1(struct sk_buff *skb, const struct xt_action_param *par) +{ + return secmark_tg(skb, par->targinfo); +} + +static struct xt_target secmark_tg_reg[] __read_mostly = { + { + .name = "SECMARK", + .revision = 0, + .family = NFPROTO_UNSPEC, + .checkentry = secmark_tg_check_v0, + .destroy = secmark_tg_destroy, + .target = secmark_tg_v0, + .targetsize = sizeof(struct xt_secmark_target_info), + .me = THIS_MODULE, + }, + { + .name = "SECMARK", + .revision = 1, + .family = NFPROTO_UNSPEC, + .checkentry = secmark_tg_check_v1, + .destroy = secmark_tg_destroy, + .target = secmark_tg_v1, + .targetsize = sizeof(struct xt_secmark_target_info_v1), + .usersize = offsetof(struct xt_secmark_target_info_v1, secid), + .me = THIS_MODULE, + }, }; static int __init secmark_tg_init(void) { - return xt_register_target(&secmark_tg_reg); + return xt_register_targets(secmark_tg_reg, ARRAY_SIZE(secmark_tg_reg)); } static void __exit secmark_tg_exit(void) { - xt_unregister_target(&secmark_tg_reg); + xt_unregister_targets(secmark_tg_reg, ARRAY_SIZE(secmark_tg_reg)); } module_init(secmark_tg_init); -- cgit v1.2.3 From 43016d02cf6e46edfc4696452251d34bba0c0435 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Mon, 3 May 2021 13:51:15 +0200 Subject: netfilter: arptables: use pernet ops struct during unregister Like with iptables and ebtables, hook unregistration has to use the pernet ops struct, not the template. This triggered following splat: hook not found, pf 3 num 0 WARNING: CPU: 0 PID: 224 at net/netfilter/core.c:480 __nf_unregister_net_hook+0x1eb/0x610 net/netfilter/core.c:480 [..] nf_unregister_net_hook net/netfilter/core.c:502 [inline] nf_unregister_net_hooks+0x117/0x160 net/netfilter/core.c:576 arpt_unregister_table_pre_exit+0x67/0x80 net/ipv4/netfilter/arp_tables.c:1565 Fixes: f9006acc8dfe5 ("netfilter: arp_tables: pass table pointer via nf_hook_ops") Reported-by: syzbot+dcccba8a1e41a38cb9df@syzkaller.appspotmail.com Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- include/linux/netfilter_arp/arp_tables.h | 3 +-- net/ipv4/netfilter/arp_tables.c | 5 ++--- net/ipv4/netfilter/arptable_filter.c | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/linux/netfilter_arp/arp_tables.h b/include/linux/netfilter_arp/arp_tables.h index 2aab9612f6ab..4f9a4b3c5892 100644 --- a/include/linux/netfilter_arp/arp_tables.h +++ b/include/linux/netfilter_arp/arp_tables.h @@ -53,8 +53,7 @@ int arpt_register_table(struct net *net, const struct xt_table *table, const struct arpt_replace *repl, const struct nf_hook_ops *ops); void arpt_unregister_table(struct net *net, const char *name); -void arpt_unregister_table_pre_exit(struct net *net, const char *name, - const struct nf_hook_ops *ops); +void arpt_unregister_table_pre_exit(struct net *net, const char *name); extern unsigned int arpt_do_table(struct sk_buff *skb, const struct nf_hook_state *state, struct xt_table *table); diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index cf20316094d0..c53f14b94356 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c @@ -1556,13 +1556,12 @@ out_free: return ret; } -void arpt_unregister_table_pre_exit(struct net *net, const char *name, - const struct nf_hook_ops *ops) +void arpt_unregister_table_pre_exit(struct net *net, const char *name) { struct xt_table *table = xt_find_table(net, NFPROTO_ARP, name); if (table) - nf_unregister_net_hooks(net, ops, hweight32(table->valid_hooks)); + nf_unregister_net_hooks(net, table->ops, hweight32(table->valid_hooks)); } EXPORT_SYMBOL(arpt_unregister_table_pre_exit); diff --git a/net/ipv4/netfilter/arptable_filter.c b/net/ipv4/netfilter/arptable_filter.c index b8f45e9bbec8..6922612df456 100644 --- a/net/ipv4/netfilter/arptable_filter.c +++ b/net/ipv4/netfilter/arptable_filter.c @@ -54,7 +54,7 @@ static int __net_init arptable_filter_table_init(struct net *net) static void __net_exit arptable_filter_net_pre_exit(struct net *net) { - arpt_unregister_table_pre_exit(net, "filter", arpfilter_ops); + arpt_unregister_table_pre_exit(net, "filter"); } static void __net_exit arptable_filter_net_exit(struct net *net) -- cgit v1.2.3 From 7072a355ba191c08b0579f0f66e3eba0e28bf818 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Wed, 5 May 2021 00:33:24 -0700 Subject: netfilter: nfnetlink: add a missing rcu_read_unlock() Reported by syzbot : BUG: sleeping function called from invalid context at include/linux/sched/mm.h:201 in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 26899, name: syz-executor.5 1 lock held by syz-executor.5/26899: #0: ffffffff8bf797a0 (rcu_read_lock){....}-{1:2}, at: nfnetlink_get_subsys net/netfilter/nfnetlink.c:148 [inline] #0: ffffffff8bf797a0 (rcu_read_lock){....}-{1:2}, at: nfnetlink_rcv_msg+0x1da/0x1300 net/netfilter/nfnetlink.c:226 Preemption disabled at: [] preempt_schedule_irq+0x3e/0x90 kernel/sched/core.c:5533 CPU: 1 PID: 26899 Comm: syz-executor.5 Not tainted 5.12.0-next-20210504-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:79 [inline] dump_stack+0x141/0x1d7 lib/dump_stack.c:120 ___might_sleep.cold+0x1f1/0x237 kernel/sched/core.c:8338 might_alloc include/linux/sched/mm.h:201 [inline] slab_pre_alloc_hook mm/slab.h:500 [inline] slab_alloc_node mm/slub.c:2845 [inline] kmem_cache_alloc_node+0x33d/0x3e0 mm/slub.c:2960 __alloc_skb+0x20b/0x340 net/core/skbuff.c:413 alloc_skb include/linux/skbuff.h:1107 [inline] nlmsg_new include/net/netlink.h:953 [inline] netlink_ack+0x1ed/0xaa0 net/netlink/af_netlink.c:2437 netlink_rcv_skb+0x33d/0x420 net/netlink/af_netlink.c:2508 nfnetlink_rcv+0x1ac/0x420 net/netfilter/nfnetlink.c:650 netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 sock_sendmsg_nosec net/socket.c:654 [inline] sock_sendmsg+0xcf/0x120 net/socket.c:674 ____sys_sendmsg+0x6e8/0x810 net/socket.c:2350 ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 do_syscall_64+0x3a/0xb0 arch/x86/entry/common.c:47 entry_SYSCALL_64_after_hwframe+0x44/0xae RIP: 0033:0x4665f9 Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007fa8a03ee188 EFLAGS: 00000246 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 000000000056bf60 RCX: 00000000004665f9 RDX: 0000000000000000 RSI: 0000000020000480 RDI: 0000000000000004 RBP: 00000000004bfce1 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 000000000056bf60 R13: 00007fffe864480f R14: 00007fa8a03ee300 R15: 0000000000022000 ================================================ WARNING: lock held when returning to user space! 5.12.0-next-20210504-syzkaller #0 Tainted: G W ------------------------------------------------ syz-executor.5/26899 is leaving the kernel with locks still held! 1 lock held by syz-executor.5/26899: #0: ffffffff8bf797a0 (rcu_read_lock){....}-{1:2}, at: nfnetlink_get_subsys net/netfilter/nfnetlink.c:148 [inline] #0: ffffffff8bf797a0 (rcu_read_lock){....}-{1:2}, at: nfnetlink_rcv_msg+0x1da/0x1300 net/netfilter/nfnetlink.c:226 ------------[ cut here ]------------ WARNING: CPU: 0 PID: 26899 at kernel/rcu/tree_plugin.h:359 rcu_note_context_switch+0xfd/0x16e0 kernel/rcu/tree_plugin.h:359 Modules linked in: CPU: 0 PID: 26899 Comm: syz-executor.5 Tainted: G W 5.12.0-next-20210504-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:rcu_note_context_switch+0xfd/0x16e0 kernel/rcu/tree_plugin.h:359 Code: 48 89 fa 48 c1 ea 03 0f b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 2e 0d 00 00 8b bd cc 03 00 00 85 ff 7e 02 <0f> 0b 65 48 8b 2c 25 00 f0 01 00 48 8d bd cc 03 00 00 48 b8 00 00 RSP: 0000:ffffc90002fffdb0 EFLAGS: 00010002 RAX: 0000000000000007 RBX: ffff8880b9c36080 RCX: ffffffff8dc99bac RDX: 0000000000000000 RSI: 0000000000000008 RDI: 0000000000000001 RBP: ffff88808b9d1c80 R08: 0000000000000000 R09: ffffffff8dc96917 R10: fffffbfff1b92d22 R11: 0000000000000000 R12: 0000000000000000 R13: ffff88808b9d1c80 R14: ffff88808b9d1c80 R15: ffffc90002ff8000 FS: 00007fa8a03ee700(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f09896ed000 CR3: 0000000032070000 CR4: 00000000001526f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: __schedule+0x214/0x23e0 kernel/sched/core.c:5044 schedule+0xcf/0x270 kernel/sched/core.c:5226 exit_to_user_mode_loop kernel/entry/common.c:162 [inline] exit_to_user_mode_prepare+0x13e/0x280 kernel/entry/common.c:208 irqentry_exit_to_user_mode+0x5/0x40 kernel/entry/common.c:314 asm_sysvec_reschedule_ipi+0x12/0x20 arch/x86/include/asm/idtentry.h:637 RIP: 0033:0x4665f9 Fixes: 50f2db9e368f ("netfilter: nfnetlink: consolidate callback types") Signed-off-by: Eric Dumazet Reported-by: syzbot Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nfnetlink.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c index d7a9628b6cee..e8dbd8379027 100644 --- a/net/netfilter/nfnetlink.c +++ b/net/netfilter/nfnetlink.c @@ -295,6 +295,7 @@ replay: nfnl_unlock(subsys_id); break; default: + rcu_read_unlock(); err = -EINVAL; break; } -- cgit v1.2.3 From 5e024c325406470d1165a09c6feaf8ec897936be Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Wed, 5 May 2021 22:25:24 +0200 Subject: netfilter: nfnetlink_osf: Fix a missing skb_header_pointer() NULL check Do not assume that the tcph->doff field is correct when parsing for TCP options, skb_header_pointer() might fail to fetch these bits. Fixes: 11eeef41d5f6 ("netfilter: passive OS fingerprint xtables match") Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nfnetlink_osf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c index e8f8875c6884..0fa2e2030427 100644 --- a/net/netfilter/nfnetlink_osf.c +++ b/net/netfilter/nfnetlink_osf.c @@ -186,6 +186,8 @@ static const struct tcphdr *nf_osf_hdr_ctx_init(struct nf_osf_hdr_ctx *ctx, ctx->optp = skb_header_pointer(skb, ip_hdrlen(skb) + sizeof(struct tcphdr), ctx->optsize, opts); + if (!ctx->optp) + return NULL; } return tcp; -- cgit v1.2.3 From 198ad973839ca4686f3575155ba9ff178289905f Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Wed, 5 May 2021 22:30:49 +0200 Subject: netfilter: remove BUG_ON() after skb_header_pointer() Several conntrack helpers and the TCP tracker assume that skb_header_pointer() never fails based on upfront header validation. Even if this should not ever happen, BUG_ON() is a too drastic measure, remove them. Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_conntrack_ftp.c | 5 ++++- net/netfilter/nf_conntrack_h323_main.c | 3 ++- net/netfilter/nf_conntrack_irc.c | 5 ++++- net/netfilter/nf_conntrack_pptp.c | 4 +++- net/netfilter/nf_conntrack_proto_tcp.c | 6 ++++-- net/netfilter/nf_conntrack_sane.c | 5 ++++- 6 files changed, 21 insertions(+), 7 deletions(-) diff --git a/net/netfilter/nf_conntrack_ftp.c b/net/netfilter/nf_conntrack_ftp.c index b22801f97bce..a414274338cf 100644 --- a/net/netfilter/nf_conntrack_ftp.c +++ b/net/netfilter/nf_conntrack_ftp.c @@ -413,7 +413,10 @@ static int help(struct sk_buff *skb, spin_lock_bh(&nf_ftp_lock); fb_ptr = skb_header_pointer(skb, dataoff, datalen, ftp_buffer); - BUG_ON(fb_ptr == NULL); + if (!fb_ptr) { + spin_unlock_bh(&nf_ftp_lock); + return NF_ACCEPT; + } ends_in_nl = (fb_ptr[datalen - 1] == '\n'); seq = ntohl(th->seq) + datalen; diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c index 8ba037b76ad3..aafaff00baf1 100644 --- a/net/netfilter/nf_conntrack_h323_main.c +++ b/net/netfilter/nf_conntrack_h323_main.c @@ -146,7 +146,8 @@ static int get_tpkt_data(struct sk_buff *skb, unsigned int protoff, /* Get first TPKT pointer */ tpkt = skb_header_pointer(skb, tcpdataoff, tcpdatalen, h323_buffer); - BUG_ON(tpkt == NULL); + if (!tpkt) + goto clear_out; /* Validate TPKT identifier */ if (tcpdatalen < 4 || tpkt[0] != 0x03 || tpkt[1] != 0) { diff --git a/net/netfilter/nf_conntrack_irc.c b/net/netfilter/nf_conntrack_irc.c index e40988a2f22f..08ee4e760a3d 100644 --- a/net/netfilter/nf_conntrack_irc.c +++ b/net/netfilter/nf_conntrack_irc.c @@ -143,7 +143,10 @@ static int help(struct sk_buff *skb, unsigned int protoff, spin_lock_bh(&irc_buffer_lock); ib_ptr = skb_header_pointer(skb, dataoff, skb->len - dataoff, irc_buffer); - BUG_ON(ib_ptr == NULL); + if (!ib_ptr) { + spin_unlock_bh(&irc_buffer_lock); + return NF_ACCEPT; + } data = ib_ptr; data_limit = ib_ptr + skb->len - dataoff; diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c index 5105d4250012..7d5708b92138 100644 --- a/net/netfilter/nf_conntrack_pptp.c +++ b/net/netfilter/nf_conntrack_pptp.c @@ -544,7 +544,9 @@ conntrack_pptp_help(struct sk_buff *skb, unsigned int protoff, nexthdr_off = protoff; tcph = skb_header_pointer(skb, nexthdr_off, sizeof(_tcph), &_tcph); - BUG_ON(!tcph); + if (!tcph) + return NF_ACCEPT; + nexthdr_off += tcph->doff * 4; datalen = tcplen - tcph->doff * 4; diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c index 318b8f723349..34e22416a721 100644 --- a/net/netfilter/nf_conntrack_proto_tcp.c +++ b/net/netfilter/nf_conntrack_proto_tcp.c @@ -338,7 +338,8 @@ static void tcp_options(const struct sk_buff *skb, ptr = skb_header_pointer(skb, dataoff + sizeof(struct tcphdr), length, buff); - BUG_ON(ptr == NULL); + if (!ptr) + return; state->td_scale = state->flags = 0; @@ -394,7 +395,8 @@ static void tcp_sack(const struct sk_buff *skb, unsigned int dataoff, ptr = skb_header_pointer(skb, dataoff + sizeof(struct tcphdr), length, buff); - BUG_ON(ptr == NULL); + if (!ptr) + return; /* Fast path for timestamp-only option */ if (length == TCPOLEN_TSTAMP_ALIGNED diff --git a/net/netfilter/nf_conntrack_sane.c b/net/netfilter/nf_conntrack_sane.c index 1aebd6569d4e..fcb33b1d5456 100644 --- a/net/netfilter/nf_conntrack_sane.c +++ b/net/netfilter/nf_conntrack_sane.c @@ -95,7 +95,10 @@ static int help(struct sk_buff *skb, spin_lock_bh(&nf_sane_lock); sb_ptr = skb_header_pointer(skb, dataoff, datalen, sane_buffer); - BUG_ON(sb_ptr == NULL); + if (!sb_ptr) { + spin_unlock_bh(&nf_sane_lock); + return NF_ACCEPT; + } if (dir == IP_CT_DIR_ORIGINAL) { if (datalen != sizeof(struct sane_request)) -- cgit v1.2.3 From 85dfd816fabfc16e71786eda0a33a7046688b5b0 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Wed, 5 May 2021 23:06:43 +0200 Subject: netfilter: nftables: Fix a memleak from userdata error path in new objects Release object name if userdata allocation fails. Fixes: b131c96496b3 ("netfilter: nf_tables: add userdata support for nft_object") Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_tables_api.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 0b7fe0a902ff..926da6ed8d51 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -6615,9 +6615,9 @@ err_obj_ht: INIT_LIST_HEAD(&obj->list); return err; err_trans: - kfree(obj->key.name); -err_userdata: kfree(obj->udata); +err_userdata: + kfree(obj->key.name); err_strdup: if (obj->ops->destroy) obj->ops->destroy(&ctx, obj); -- cgit v1.2.3 From a54754ec9891830ba548e2010c889e3c8146e449 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 6 May 2021 05:53:23 -0700 Subject: netfilter: nftables: avoid overflows in nft_hash_buckets() Number of buckets being stored in 32bit variables, we have to ensure that no overflows occur in nft_hash_buckets() syzbot injected a size == 0x40000000 and reported: UBSAN: shift-out-of-bounds in ./include/linux/log2.h:57:13 shift exponent 64 is too large for 64-bit type 'long unsigned int' CPU: 1 PID: 29539 Comm: syz-executor.4 Not tainted 5.12.0-rc7-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:79 [inline] dump_stack+0x141/0x1d7 lib/dump_stack.c:120 ubsan_epilogue+0xb/0x5a lib/ubsan.c:148 __ubsan_handle_shift_out_of_bounds.cold+0xb1/0x181 lib/ubsan.c:327 __roundup_pow_of_two include/linux/log2.h:57 [inline] nft_hash_buckets net/netfilter/nft_set_hash.c:411 [inline] nft_hash_estimate.cold+0x19/0x1e net/netfilter/nft_set_hash.c:652 nft_select_set_ops net/netfilter/nf_tables_api.c:3586 [inline] nf_tables_newset+0xe62/0x3110 net/netfilter/nf_tables_api.c:4322 nfnetlink_rcv_batch+0xa09/0x24b0 net/netfilter/nfnetlink.c:488 nfnetlink_rcv_skb_batch net/netfilter/nfnetlink.c:612 [inline] nfnetlink_rcv+0x3af/0x420 net/netfilter/nfnetlink.c:630 netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 sock_sendmsg_nosec net/socket.c:654 [inline] sock_sendmsg+0xcf/0x120 net/socket.c:674 ____sys_sendmsg+0x6e8/0x810 net/socket.c:2350 ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 Fixes: 0ed6389c483d ("netfilter: nf_tables: rename set implementations") Signed-off-by: Eric Dumazet Reported-by: syzbot Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nft_set_hash.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c index 58f576abcd4a..328f2ce32e4c 100644 --- a/net/netfilter/nft_set_hash.c +++ b/net/netfilter/nft_set_hash.c @@ -412,9 +412,17 @@ static void nft_rhash_destroy(const struct nft_set *set) (void *)set); } +/* Number of buckets is stored in u32, so cap our result to 1U<<31 */ +#define NFT_MAX_BUCKETS (1U << 31) + static u32 nft_hash_buckets(u32 size) { - return roundup_pow_of_two(size * 4 / 3); + u64 val = div_u64((u64)size * 4, 3); + + if (val >= NFT_MAX_BUCKETS) + return NFT_MAX_BUCKETS; + + return roundup_pow_of_two(val); } static bool nft_rhash_estimate(const struct nft_set_desc *desc, u32 features, -- cgit v1.2.3 From 6c8774a94e6ad26f29ef103c8671f55c255c6201 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 6 May 2021 05:53:50 -0700 Subject: netfilter: nftables: avoid potential overflows on 32bit arches User space could ask for very large hash tables, we need to make sure our size computations wont overflow. nf_tables_newset() needs to double check the u64 size will fit into size_t field. Fixes: 0ed6389c483d ("netfilter: nf_tables: rename set implementations") Signed-off-by: Eric Dumazet Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_tables_api.c | 7 +++++-- net/netfilter/nft_set_hash.c | 10 +++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 926da6ed8d51..d63d2d8f769c 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -4184,6 +4184,7 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info, unsigned char *udata; struct nft_set *set; struct nft_ctx ctx; + size_t alloc_size; u64 timeout; char *name; int err, i; @@ -4329,8 +4330,10 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info, size = 0; if (ops->privsize != NULL) size = ops->privsize(nla, &desc); - - set = kvzalloc(sizeof(*set) + size + udlen, GFP_KERNEL); + alloc_size = sizeof(*set) + size + udlen; + if (alloc_size < size) + return -ENOMEM; + set = kvzalloc(alloc_size, GFP_KERNEL); if (!set) return -ENOMEM; diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c index 328f2ce32e4c..7b3d0a78c569 100644 --- a/net/netfilter/nft_set_hash.c +++ b/net/netfilter/nft_set_hash.c @@ -623,7 +623,7 @@ static u64 nft_hash_privsize(const struct nlattr * const nla[], const struct nft_set_desc *desc) { return sizeof(struct nft_hash) + - nft_hash_buckets(desc->size) * sizeof(struct hlist_head); + (u64)nft_hash_buckets(desc->size) * sizeof(struct hlist_head); } static int nft_hash_init(const struct nft_set *set, @@ -663,8 +663,8 @@ static bool nft_hash_estimate(const struct nft_set_desc *desc, u32 features, return false; est->size = sizeof(struct nft_hash) + - nft_hash_buckets(desc->size) * sizeof(struct hlist_head) + - desc->size * sizeof(struct nft_hash_elem); + (u64)nft_hash_buckets(desc->size) * sizeof(struct hlist_head) + + (u64)desc->size * sizeof(struct nft_hash_elem); est->lookup = NFT_SET_CLASS_O_1; est->space = NFT_SET_CLASS_O_N; @@ -681,8 +681,8 @@ static bool nft_hash_fast_estimate(const struct nft_set_desc *desc, u32 features return false; est->size = sizeof(struct nft_hash) + - nft_hash_buckets(desc->size) * sizeof(struct hlist_head) + - desc->size * sizeof(struct nft_hash_elem); + (u64)nft_hash_buckets(desc->size) * sizeof(struct hlist_head) + + (u64)desc->size * sizeof(struct nft_hash_elem); est->lookup = NFT_SET_CLASS_O_1; est->space = NFT_SET_CLASS_O_N; -- cgit v1.2.3