All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] ethtool: limit bitset size
@ 2020-02-24 19:42 Michal Kubecek
  2020-02-26 19:28 ` David Miller
  0 siblings, 1 reply; 2+ messages in thread
From: Michal Kubecek @ 2020-02-24 19:42 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev; +Cc: linux-kernel

Syzbot reported that ethnl_compact_sanity_checks() can be tricked into
reading past the end of ETHTOOL_A_BITSET_VALUE and ETHTOOL_A_BITSET_MASK
attributes and even the message by passing a value between (u32)(-31)
and (u32)(-1) as ETHTOOL_A_BITSET_SIZE.

The problem is that DIV_ROUND_UP(attr_nbits, 32) is 0 for such values so
that zero length ETHTOOL_A_BITSET_VALUE will pass the length check but
ethnl_bitmap32_not_zero() check would try to access up to 512 MB of
attribute "payload".

Prevent this overflow byt limiting the bitset size. Technically, compact
bitset format would allow bitset sizes up to almost 2^18 (so that the
nest size does not exceed U16_MAX) but bitsets used by ethtool are much
shorter. S16_MAX, the largest value which can be directly used as an
upper limit in policy, should be a reasonable compromise.

Fixes: 10b518d4e6dd ("ethtool: netlink bitset handling")
Reported-by: syzbot+7fd4ed5b4234ab1fdccd@syzkaller.appspotmail.com
Reported-by: syzbot+709b7a64d57978247e44@syzkaller.appspotmail.com
Reported-by: syzbot+983cb8fb2d17a7af549d@syzkaller.appspotmail.com
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 net/ethtool/bitset.c | 3 ++-
 net/ethtool/bitset.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ethtool/bitset.c b/net/ethtool/bitset.c
index 8977fe1f3946..ef9197541cb3 100644
--- a/net/ethtool/bitset.c
+++ b/net/ethtool/bitset.c
@@ -305,7 +305,8 @@ int ethnl_put_bitset32(struct sk_buff *skb, int attrtype, const u32 *val,
 static const struct nla_policy bitset_policy[ETHTOOL_A_BITSET_MAX + 1] = {
 	[ETHTOOL_A_BITSET_UNSPEC]	= { .type = NLA_REJECT },
 	[ETHTOOL_A_BITSET_NOMASK]	= { .type = NLA_FLAG },
-	[ETHTOOL_A_BITSET_SIZE]		= { .type = NLA_U32 },
+	[ETHTOOL_A_BITSET_SIZE]		= NLA_POLICY_MAX(NLA_U32,
+							 ETHNL_MAX_BITSET_SIZE),
 	[ETHTOOL_A_BITSET_BITS]		= { .type = NLA_NESTED },
 	[ETHTOOL_A_BITSET_VALUE]	= { .type = NLA_BINARY },
 	[ETHTOOL_A_BITSET_MASK]		= { .type = NLA_BINARY },
diff --git a/net/ethtool/bitset.h b/net/ethtool/bitset.h
index b8247e34109d..b849f9d19676 100644
--- a/net/ethtool/bitset.h
+++ b/net/ethtool/bitset.h
@@ -3,6 +3,8 @@
 #ifndef _NET_ETHTOOL_BITSET_H
 #define _NET_ETHTOOL_BITSET_H
 
+#define ETHNL_MAX_BITSET_SIZE S16_MAX
+
 typedef const char (*const ethnl_string_array_t)[ETH_GSTRING_LEN];
 
 int ethnl_bitset_is_compact(const struct nlattr *bitset, bool *compact);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] ethtool: limit bitset size
  2020-02-24 19:42 [PATCH net] ethtool: limit bitset size Michal Kubecek
@ 2020-02-26 19:28 ` David Miller
  0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2020-02-26 19:28 UTC (permalink / raw)
  To: mkubecek; +Cc: kuba, netdev, linux-kernel

From: Michal Kubecek <mkubecek@suse.cz>
Date: Mon, 24 Feb 2020 20:42:12 +0100 (CET)

> Syzbot reported that ethnl_compact_sanity_checks() can be tricked into
> reading past the end of ETHTOOL_A_BITSET_VALUE and ETHTOOL_A_BITSET_MASK
> attributes and even the message by passing a value between (u32)(-31)
> and (u32)(-1) as ETHTOOL_A_BITSET_SIZE.
> 
> The problem is that DIV_ROUND_UP(attr_nbits, 32) is 0 for such values so
> that zero length ETHTOOL_A_BITSET_VALUE will pass the length check but
> ethnl_bitmap32_not_zero() check would try to access up to 512 MB of
> attribute "payload".
> 
> Prevent this overflow byt limiting the bitset size. Technically, compact
> bitset format would allow bitset sizes up to almost 2^18 (so that the
> nest size does not exceed U16_MAX) but bitsets used by ethtool are much
> shorter. S16_MAX, the largest value which can be directly used as an
> upper limit in policy, should be a reasonable compromise.
> 
> Fixes: 10b518d4e6dd ("ethtool: netlink bitset handling")
> Reported-by: syzbot+7fd4ed5b4234ab1fdccd@syzkaller.appspotmail.com
> Reported-by: syzbot+709b7a64d57978247e44@syzkaller.appspotmail.com
> Reported-by: syzbot+983cb8fb2d17a7af549d@syzkaller.appspotmail.com
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Applied, thanks Michal.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-02-26 19:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 19:42 [PATCH net] ethtool: limit bitset size Michal Kubecek
2020-02-26 19:28 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.