From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kees Cook Subject: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL Date: Sat, 25 Aug 2018 22:58:01 -0700 Message-ID: <20180826055801.GA42063@beast> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Al Viro , Jamal Hadi Salim , Cong Wang , Jiri Pirko , "David S. Miller" , netdev@vger.kernel.org To: linux-kernel@vger.kernel.org Return-path: Received: from mail-pg1-f195.google.com ([209.85.215.195]:44016 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726254AbeHZJjb (ORCPT ); Sun, 26 Aug 2018 05:39:31 -0400 Received: by mail-pg1-f195.google.com with SMTP id v66-v6so6001147pgb.10 for ; Sat, 25 Aug 2018 22:58:04 -0700 (PDT) Content-Disposition: inline Sender: netdev-owner@vger.kernel.org List-ID: 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 Cc: Jamal Hadi Salim Cc: Cong Wang Cc: Jiri Pirko Cc: "David S. Miller" Cc: netdev@vger.kernel.org Signed-off-by: Kees Cook --- This should go through -stable please, but I have left off the "Cc: stable" as per netdev patch policy. Note that use of struct_size() will need manual expansion in backports, such as: sel_size = sizeof(*s) + sizeof(*s->keys) * s->nkeys; --- net/sched/cls_u32.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index d5d2a6dc3921..f218ccf1e2d9 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -914,6 +914,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, flags = 0; + size_t sel_size; int err; #ifdef CONFIG_CLS_U32_PERF size_t size; @@ -1076,8 +1077,13 @@ static int u32_change(struct net *net, struct sk_buff *in_skb, } s = nla_data(tb[TCA_U32_SEL]); + sel_size = struct_size(s, keys, s->nkeys); + if (nla_len(tb[TCA_U32_SEL]) < sel_size) { + err = -EINVAL; + goto erridr; + } - 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) { err = -ENOBUFS; goto erridr; @@ -1092,7 +1098,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; -- 2.17.1 -- Kees Cook Pixel Security