From: Kees Cook keescook@chromium.org

commit 98c8f125fd8a6240ea343c1aa50a1be9047791b8 upstream

Via u32_change(), TCA_U32_SEL has an unspecified type in the netlink policy, so max length isn't enforced, only minimum. This means nkeys (from userspace) was being trusted without checking the actual size of nla_len(), which could lead to a memory over-read, and ultimately an exposure via a call to u32_dump(). Reachability is CAP_NET_ADMIN within a namespace.

Reported-by: Al Viro viro@zeniv.linux.org.uk Cc: Jamal Hadi Salim jhs@mojatatu.com Cc: Cong Wang xiyou.wangcong@gmail.com Cc: Jiri Pirko jiri@resnulli.us Cc: "David S. Miller" davem@davemloft.net Cc: netdev@vger.kernel.org Signed-off-by: Kees Cook keescook@chromium.org Acked-by: Jamal Hadi Salim jhs@mojatatu.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Zubin Mithra zsm@chromium.org --- Notes: * Syzkaller triggered an OOB read in u32_change with the following stacktrace: [<ffffffff81cb8911>] __dump_stack lib/dump_stack.c:15 [inline] [<ffffffff81cb8911>] dump_stack+0xc1/0x120 lib/dump_stack.c:51 [<ffffffff81525640>] print_trailer+0x151/0x15a mm/slub.c:658 [<ffffffff81525913>] object_err+0x35/0x3d mm/slub.c:665 [<ffffffff8152619f>] print_address_description mm/kasan/report.c:188 [inline] [<ffffffff8152619f>] kasan_report_error mm/kasan/report.c:285 [inline] [<ffffffff8152619f>] kasan_report.part.0.cold+0x21a/0x4c1 mm/kasan/report.c:310 [<ffffffff814ec0a5>] kasan_report+0x25/0x30 mm/kasan/report.c:297 [<ffffffff814eb336>] check_memory_region_inline mm/kasan/kasan.c:292 [inline] [<ffffffff814eb336>] check_memory_region+0x116/0x190 mm/kasan/kasan.c:299 [<ffffffff814eb7d4>] memcpy+0x24/0x50 mm/kasan/kasan.c:334 [<ffffffff823b6d33>] u32_change+0xb03/0x1e60 net/sched/cls_u32.c:844 [<ffffffff8232f1a0>] tc_ctl_tfilter+0xc20/0x15f0 net/sched/cls_api.c:335 [<ffffffff822d82e5>] rtnetlink_rcv_msg+0x4f5/0x6d0 net/core/rtnetlink.c:3605 [<ffffffff823c92ef>] netlink_rcv_skb+0xdf/0x2f0 net/netlink/af_netlink.c:2361 [<ffffffff822d7de0>] rtnetlink_rcv+0x30/0x40 net/core/rtnetlink.c:3611 [<ffffffff823c7f02>] netlink_unicast_kernel net/netlink/af_netlink.c:1277 [inline] [<ffffffff823c7f02>] netlink_unicast+0x462/0x650 net/netlink/af_netlink.c:1303 [<ffffffff823c8891>] netlink_sendmsg+0x7a1/0xc50 net/netlink/af_netlink.c:1859 [<ffffffff8223b165>] sock_sendmsg_nosec net/socket.c:627 [inline] [<ffffffff8223b165>] sock_sendmsg+0xd5/0x110 net/socket.c:637 [<ffffffff8223ebe7>] ___sys_sendmsg+0x767/0x890 net/socket.c:1964 [<ffffffff8224175b>] __sys_sendmsg+0xbb/0x150 net/socket.c:1998 [<ffffffff82241822>] SYSC_sendmsg net/socket.c:2009 [inline] [<ffffffff82241822>] SyS_sendmsg+0x32/0x50 net/socket.c:2005 [<ffffffff82a489e7>] entry_SYSCALL_64_fastpath+0x1e/0xa0

* The commit is present in linux-4.9.y.

* This backport is similar to the commit in linux-4.9.y and addresses conflicts related to struct_size() not being present.

* Tests run: Chrome OS tryjobs, Syzkaller reproducer

net/sched/cls_u32.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 4fbb67430ce48..4d745a2efd201 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -734,6 +734,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb, struct nlattr *opt = tca[TCA_OPTIONS]; struct nlattr *tb[TCA_U32_MAX + 1]; u32 htid; + size_t sel_size; int err; #ifdef CONFIG_CLS_U32_PERF size_t size; @@ -827,8 +828,11 @@ static int u32_change(struct net *net, struct sk_buff *in_skb, return -EINVAL;

s = nla_data(tb[TCA_U32_SEL]); + sel_size = sizeof(*s) + sizeof(*s->keys) * s->nkeys; + if (nla_len(tb[TCA_U32_SEL]) < sel_size) + return -EINVAL;

- n = kzalloc(sizeof(*n) + s->nkeys*sizeof(struct tc_u32_key), GFP_KERNEL); + n = kzalloc(offsetof(typeof(*n), sel) + sel_size, GFP_KERNEL); if (n == NULL) return -ENOBUFS;

@@ -841,7 +845,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb, } #endif

- memcpy(&n->sel, s, sizeof(*s) + s->nkeys*sizeof(struct tc_u32_key)); + memcpy(&n->sel, s, sel_size); RCU_INIT_POINTER(n->ht_up, ht); n->handle = handle; n->fshift = s->hmask ? ffs(ntohl(s->hmask)) - 1 : 0;