[PATCH wpan 00/17] ieee802154: syzbot fixes

Hi,

this patch series contains fixes found by syzbot for nl802154 and a
memory leak each time we receiving a skb for monitor interfaces.

The first three patches are misc fixes, all others are to forbid monitor
interfaces to access security mib values which are never initialized for
monitor interfaces yet. We never supported such handling but I can
imagine that we can use security mib for monitor interfaces to decrypt
802.15.4 frames by the Linux kernel and the RAW sockets can see
plaintext then. However it's a possibility for an new feature to check in
due courses.

- Alex

Alexander Aring (17):
  net: ieee802154: make shift exponent unsigned
  net: ieee802154: fix memory leak when deliver monitor skbs
  net: ieee802154: nl-mac: fix check on panid
  net: ieee802154: forbid monitor for set llsec params
  net: ieee802154: stop dump llsec keys for monitors
  net: ieee802154: forbid monitor for add llsec key
  net: ieee802154: forbid monitor for del llsec key
  net: ieee802154: stop dump llsec devs for monitors
  net: ieee802154: forbid monitor for add llsec dev
  net: ieee802154: forbid monitor for del llsec dev
  net: ieee802154: stop dump llsec devkeys for monitors
  net: ieee802154: forbid monitor for add llsec devkey
  net: ieee802154: forbid monitor for del llsec devkey
  net: ieee802154: stop dump llsec seclevels for monitors
  net: ieee802154: forbid monitor for add llsec seclevel
  net: ieee802154: forbid monitor for del llsec seclevel
  net: ieee802154: stop dump llsec params for monitors

 net/ieee802154/nl-mac.c   |  7 ++---
 net/ieee802154/nl802154.c | 54 ++++++++++++++++++++++++++++++++++++++-
 net/mac802154/rx.c        |  2 ++
 3 files changed, 59 insertions(+), 4 deletions(-)

-- 
2.26.2

This patch changes the iftype type variable to unsigned that it can
never be reach a negative value.

Reported-by: syzbot+7bf7b22759195c9a21e9@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index e9e4652cd592..3ee09f6d13b7 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -898,8 +898,8 @@ static int nl802154_get_interface(struct sk_buff *skb, struct genl_info *info)
 static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg802154_registered_device *rdev = info->user_ptr[0];
-	enum nl802154_iftype type = NL802154_IFTYPE_UNSPEC;
 	__le64 extended_addr = cpu_to_le64(0x0000000000000000ULL);
+	u32 type = NL802154_IFTYPE_UNSPEC;
 
 	/* TODO avoid failing a new interface
 	 * creation due to pending removal?

-- 
2.26.2

This patch fixes a null pointer derefence for panid handle by move the
check for the netlink variable directly before accessing them.

Reported-by: syzbot+d4c07de0144f6f63be3a@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl-mac.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ieee802154/nl-mac.c b/net/ieee802154/nl-mac.c
index 9c640d670ffe..0c1b0770c59e 100644
--- a/net/ieee802154/nl-mac.c
+++ b/net/ieee802154/nl-mac.c

@@ -551,9 +551,7 @@ ieee802154_llsec_parse_key_id(struct genl_info *info,
 	desc->mode = nla_get_u8(info->attrs[IEEE802154_ATTR_LLSEC_KEY_MODE]);
 
 	if (desc->mode == IEEE802154_SCF_KEY_IMPLICIT) {
-		if (!info->attrs[IEEE802154_ATTR_PAN_ID] &&
-		    !(info->attrs[IEEE802154_ATTR_SHORT_ADDR] ||
-		      info->attrs[IEEE802154_ATTR_HW_ADDR]))
+		if (!info->attrs[IEEE802154_ATTR_PAN_ID])
 			return -EINVAL;
 
 		desc->device_addr.pan_id = nla_get_shortaddr(info->attrs[IEEE802154_ATTR_PAN_ID]);

@@ -562,6 +560,9 @@ ieee802154_llsec_parse_key_id(struct genl_info *info,
 			desc->device_addr.mode = IEEE802154_ADDR_SHORT;
 			desc->device_addr.short_addr = nla_get_shortaddr(info->attrs[IEEE802154_ATTR_SHORT_ADDR]);
 		} else {
+			if (!info->attrs[IEEE802154_ATTR_HW_ADDR])
+				return -EINVAL;
+
 			desc->device_addr.mode = IEEE802154_ADDR_LONG;
 			desc->device_addr.extended_addr = nla_get_hwaddr(info->attrs[IEEE802154_ATTR_HW_ADDR]);
 		}

-- 
2.26.2

This patch stops dumping llsec keys for monitors which we don't support
yet. Otherwise we will access llsec mib which isn't initialized for
monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 67f0dc622bc2..f8f1792c3620 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1493,6 +1493,11 @@ nl802154_dump_llsec_key(struct sk_buff *skb, struct netlink_callback *cb)
 	if (err)
 		return err;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) {
+		err = skb->len;
+		goto out_err;
+	}
+
 	if (!wpan_dev->netdev) {
 		err = -EINVAL;
 		goto out_err;

-- 
2.26.2

This patch forbids to add llsec key for monitor interfaces which we
don't support yet. Otherwise we will access llsec mib which isn't
initialized for monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index f8f1792c3620..3bc71505ad63 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1552,6 +1552,9 @@ static int nl802154_add_llsec_key(struct sk_buff *skb, struct genl_info *info)
 	struct ieee802154_llsec_key_id id = { };
 	u32 commands[NL802154_CMD_FRAME_NR_IDS / 32] = { };
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (!info->attrs[NL802154_ATTR_SEC_KEY] ||
 	    nla_parse_nested_deprecated(attrs, NL802154_KEY_ATTR_MAX, info->attrs[NL802154_ATTR_SEC_KEY], nl802154_key_policy, info->extack))
 		return -EINVAL;

-- 
2.26.2

This patch adds a missing consume_skb() when deliver a skb to upper
monitor interfaces of a wpan phy.

Reported-by: syzbot+44b651863a17760a893b@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/mac802154/rx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index b8ce84618a55..18abc1f49323 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c

@@ -244,6 +244,8 @@ ieee802154_monitors_rx(struct ieee802154_local *local, struct sk_buff *skb)
 			sdata->dev->stats.rx_bytes += skb->len;
 		}
 	}
+
+	consume_skb(skb);
 }
 
 void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb)

-- 
2.26.2

This patch forbids to del llsec key for monitor interfaces which we
don't support yet. Otherwise we will access llsec mib which isn't
initialized for monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 3bc71505ad63..fdea9c20cd8e 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1604,6 +1604,9 @@ static int nl802154_del_llsec_key(struct sk_buff *skb, struct genl_info *info)
 	struct nlattr *attrs[NL802154_KEY_ATTR_MAX + 1];
 	struct ieee802154_llsec_key_id id;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (!info->attrs[NL802154_ATTR_SEC_KEY] ||
 	    nla_parse_nested_deprecated(attrs, NL802154_KEY_ATTR_MAX, info->attrs[NL802154_ATTR_SEC_KEY], nl802154_key_policy, info->extack))
 		return -EINVAL;

-- 
2.26.2

This patch stops dumping llsec devs for monitors which we don't support
yet. Otherwise we will access llsec mib which isn't initialized for
monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index fdea9c20cd8e..7f728d85e9b6 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1672,6 +1672,11 @@ nl802154_dump_llsec_dev(struct sk_buff *skb, struct netlink_callback *cb)
 	if (err)
 		return err;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) {
+		err = skb->len;
+		goto out_err;
+	}
+
 	if (!wpan_dev->netdev) {
 		err = -EINVAL;
 		goto out_err;

-- 
2.26.2

This patch forbids to set llsec params for monitor interfaces which we
don't support yet.

Reported-by: syzbot+8b6719da8a04beeafcc3@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 3ee09f6d13b7..67f0dc622bc2 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1384,6 +1384,9 @@ static int nl802154_set_llsec_params(struct sk_buff *skb,
 	u32 changed = 0;
 	int ret;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (info->attrs[NL802154_ATTR_SEC_ENABLED]) {
 		u8 enabled;
 

-- 
2.26.2

This patch forbids to del llsec dev for monitor interfaces which we
don't support yet. Otherwise we will access llsec mib which isn't
initialized for monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index c32e55f961c2..46faf451f413 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1781,6 +1781,9 @@ static int nl802154_del_llsec_dev(struct sk_buff *skb, struct genl_info *info)
 	struct nlattr *attrs[NL802154_DEV_ATTR_MAX + 1];
 	__le64 extended_addr;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (!info->attrs[NL802154_ATTR_SEC_DEVICE] ||
 	    nla_parse_nested_deprecated(attrs, NL802154_DEV_ATTR_MAX, info->attrs[NL802154_ATTR_SEC_DEVICE], nl802154_dev_policy, info->extack))
 		return -EINVAL;

-- 
2.26.2

This patch stops dumping llsec devkeys for monitors which we don't support
yet. Otherwise we will access llsec mib which isn't initialized for
monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 46faf451f413..43e7b029c444 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1853,6 +1853,11 @@ nl802154_dump_llsec_devkey(struct sk_buff *skb, struct netlink_callback *cb)
 	if (err)
 		return err;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) {
+		err = skb->len;
+		goto out_err;
+	}
+
 	if (!wpan_dev->netdev) {
 		err = -EINVAL;
 		goto out_err;

-- 
2.26.2

This patch forbids to add llsec devkey for monitor interfaces which we
don't support yet. Otherwise we will access llsec mib which isn't
initialized for monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 43e7b029c444..ecdaff719b08 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1915,6 +1915,9 @@ static int nl802154_add_llsec_devkey(struct sk_buff *skb, struct genl_info *info
 	struct ieee802154_llsec_device_key key;
 	__le64 extended_addr;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (!info->attrs[NL802154_ATTR_SEC_DEVKEY] ||
 	    nla_parse_nested_deprecated(attrs, NL802154_DEVKEY_ATTR_MAX, info->attrs[NL802154_ATTR_SEC_DEVKEY], nl802154_devkey_policy, info->extack) < 0)
 		return -EINVAL;

-- 
2.26.2

This patch forbids to add llsec dev for monitor interfaces which we
don't support yet. Otherwise we will access llsec mib which isn't
initialized for monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 7f728d85e9b6..c32e55f961c2 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1763,6 +1763,9 @@ static int nl802154_add_llsec_dev(struct sk_buff *skb, struct genl_info *info)
 	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
 	struct ieee802154_llsec_device dev_desc;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (ieee802154_llsec_parse_device(info->attrs[NL802154_ATTR_SEC_DEVICE],
 					  &dev_desc) < 0)
 		return -EINVAL;

-- 
2.26.2

This patch forbids to del llsec devkey for monitor interfaces which we
don't support yet. Otherwise we will access llsec mib which isn't
initialized for monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index ecdaff719b08..d0352148e34c 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1949,6 +1949,9 @@ static int nl802154_del_llsec_devkey(struct sk_buff *skb, struct genl_info *info
 	struct ieee802154_llsec_device_key key;
 	__le64 extended_addr;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (!info->attrs[NL802154_ATTR_SEC_DEVKEY] ||
 	    nla_parse_nested_deprecated(attrs, NL802154_DEVKEY_ATTR_MAX, info->attrs[NL802154_ATTR_SEC_DEVKEY], nl802154_devkey_policy, info->extack))
 		return -EINVAL;

-- 
2.26.2

This patch stops dumping llsec seclevels for monitors which we don't
support yet. Otherwise we will access llsec mib which isn't initialized
for monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index d0352148e34c..95a13a5b75d0 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -2026,6 +2026,11 @@ nl802154_dump_llsec_seclevel(struct sk_buff *skb, struct netlink_callback *cb)
 	if (err)
 		return err;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR) {
+		err = skb->len;
+		goto out_err;
+	}
+
 	if (!wpan_dev->netdev) {
 		err = -EINVAL;
 		goto out_err;

-- 
2.26.2

This patch stops dumping llsec params for monitors which we don't support
yet. Otherwise we will access llsec mib which isn't initialized for
monitors.

Reported-by: syzbot+cde43a581a8e5f317bc2@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 576e418cf5aa..ca8e17a81a4f 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -820,8 +820,13 @@ nl802154_send_iface(struct sk_buff *msg, u32 portid, u32 seq, int flags,
 		goto nla_put_failure;
 
 #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		goto out;
+
 	if (nl802154_get_llsec_params(msg, rdev, wpan_dev) < 0)
 		goto nla_put_failure;
+
+out:
 #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */
 
 	genlmsg_end(msg, hdr);

-- 
2.26.2

This patch forbids to del llsec seclevel for monitor interfaces which we
don't support yet. Otherwise we will access llsec mib which isn't
initialized for monitors.

Reported-by: syzbot+fbf4fc11a819824e027b@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index e5181de4a77a..576e418cf5aa 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -2133,6 +2133,9 @@ static int nl802154_del_llsec_seclevel(struct sk_buff *skb,
 	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
 	struct ieee802154_llsec_seclevel sl;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (!info->attrs[NL802154_ATTR_SEC_LEVEL] ||
 	    llsec_parse_seclevel(info->attrs[NL802154_ATTR_SEC_LEVEL],
 				 &sl) < 0)

-- 
2.26.2

This patch forbids to add llsec seclevel for monitor interfaces which we
don't support yet. Otherwise we will access llsec mib which isn't
initialized for monitors.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 95a13a5b75d0..e5181de4a77a 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -2115,6 +2115,9 @@ static int nl802154_add_llsec_seclevel(struct sk_buff *skb,
 	struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
 	struct ieee802154_llsec_seclevel sl;
 
+	if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+		return -EOPNOTSUPP;
+
 	if (llsec_parse_seclevel(info->attrs[NL802154_ATTR_SEC_LEVEL],
 				 &sl) < 0)
 		return -EINVAL;

-- 
2.26.2

Hi Stefan,

On Sun, 28 Feb 2021 at 10:21, Alexander Aring [off-list ref] wrote:

Please drop this patch. It's not correct. I will look next weekend at
this one again.
The other patches should be fine, I hope.

- Alex

Hello Alex.

On 01.03.21 04:16, Alexander Aring wrote:

Hi Stefan,

On Sun, 28 Feb 2021 at 10:21, Alexander Aring [off-list ref] wrote:

quoted

This patch adds a missing consume_skb() when deliver a skb to upper
monitor interfaces of a wpan phy.

Reported-by: syzbot+44b651863a17760a893b@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
  net/mac802154/rx.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index b8ce84618a55..18abc1f49323 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c

@@ -244,6 +244,8 @@ ieee802154_monitors_rx(struct ieee802154_local *local, struct sk_buff *skb)
                         sdata->dev->stats.rx_bytes += skb->len;
                 }
         }
+
+       consume_skb(skb);

Please drop this patch. It's not correct. I will look next weekend at
this one again.
The other patches should be fine, I hope.

Thanks for the heads up. I dropped this patch and will take a look at 
the rest of the  series today or tomorrow.

regards
Stefan Schmidt

Hello Alex.

On 28.02.21 16:18, Alexander Aring wrote:

I am fine with this patch and all the rest up to 17. They just do not 
apply for me with 1 and 2 left out and only 3 applied.

Could you resend 3-17 as a series and we can discuss 1 & 2 separately?

regards
Stefan Schmidt

Hello Alex.

On 28.02.21 16:18, Alexander Aring wrote:

I am concerned about this one. Maybe you can shed some light on it.
NL802154_IFTYPE_UNSPEC is -1 which means the u32 will not hold this 
value, but something at the end of the range for u32.

There is a path (info->attrs[NL802154_ATTR_IFTYPE] is not true) where we 
put type forward to  rdev_add_virtual_intf() with its changed value but 
it would expect and enum which could hold -1 for UNSPEC.

regards
Stefan Schmidt

Hello.

On 28.02.21 16:18, Alexander Aring wrote:

This patch has been applied to the wpan tree and will be
part of the next pull request to net. Thanks!

regards
Stefan Schmidt

Hi,

On Thu, 4 Mar 2021 at 02:28, Stefan Schmidt [off-list ref] wrote:

Hello Alex.

On 28.02.21 16:18, Alexander Aring wrote:

quoted

This patch forbids to set llsec params for monitor interfaces which we
don't support yet.

Reported-by: syzbot+8b6719da8a04beeafcc3@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
  net/ieee802154/nl802154.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 3ee09f6d13b7..67f0dc622bc2 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -1384,6 +1384,9 @@ static int nl802154_set_llsec_params(struct sk_buff *skb,
      u32 changed = 0;
      int ret;

+     if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
+             return -EOPNOTSUPP;
+
      if (info->attrs[NL802154_ATTR_SEC_ENABLED]) {
              u8 enabled;

I am fine with this patch and all the rest up to 17. They just do not
apply for me with 1 and 2 left out and only 3 applied.

I am sorry, I will recheck.

Could you resend 3-17 as a series and we can discuss 1 & 2 separately?

okay.

- Alex

Hi Stefan,

On Thu, 4 Mar 2021 at 02:28, Stefan Schmidt [off-list ref] wrote:

Hello Alex.

On 28.02.21 16:18, Alexander Aring wrote:

quoted

This patch changes the iftype type variable to unsigned that it can
never be reach a negative value.

Reported-by: syzbot+7bf7b22759195c9a21e9@syzkaller.appspotmail.com
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
  net/ieee802154/nl802154.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index e9e4652cd592..3ee09f6d13b7 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c

@@ -898,8 +898,8 @@ static int nl802154_get_interface(struct sk_buff *skb, struct genl_info *info)
  static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
  {
      struct cfg802154_registered_device *rdev = info->user_ptr[0];
-     enum nl802154_iftype type = NL802154_IFTYPE_UNSPEC;
      __le64 extended_addr = cpu_to_le64(0x0000000000000000ULL);
+     u32 type = NL802154_IFTYPE_UNSPEC;

      /* TODO avoid failing a new interface
       * creation due to pending removal?

I am concerned about this one. Maybe you can shed some light on it.
NL802154_IFTYPE_UNSPEC is -1 which means the u32 will not hold this
value, but something at the end of the range for u32.

yes, ugh... it's NL802154_IFTYPE_UNSPEC = -1 only for
NL802154_IFTYPE... all others UNSPEC are 0. There is a comment there
/* for backwards compatibility TODO */. I think I did that because the
old netlink interfaces and instead of mapping new values to old values
(internally) which is bad.
Would it be 0 I think the compiler would handle it as unsigned.

There is a path (info->attrs[NL802154_ATTR_IFTYPE] is not true) where we
put type forward to  rdev_add_virtual_intf() with its changed value but
it would expect and enum which could hold -1 for UNSPEC.

It will be converted back here to -1 again? Or maybe depends on the
compiler, because it may use a different int type which the enum
values fits? I am not sure here...

In nl802154 we use u32 (netlink) for enums because the range fits,
however this isn't true for NL802154_IFTYPE_, we cannot change it
back. I think we should try to switch NL802154_IFTYPE_UNSPEC to
"(~(__u32)0)" and let start NL802154_IFTYPE_NODE = 0. Which is still
backwards compatible. Just give the compiler a note to handle it as
unsigned value and more importantly an enum where the range fits in.
It depends on the compiler, may it decide to use a signed char for
this enum, then we get problems when converting it ? After quick
research it seems we can not rely on whatever the compiler handles the
enum as signed or unsigned and that makes problems with the shift
operator "BIT(type)" and it's what this patch is trying to fix. I
would make two patches, one is making the nl802154.h changes and the
other is this patch, should be fine to handle it as enum value when we
did some max range checks.

There is also a third patch to return -EINVAL earlier if type attr
isn't given, I think it's nothing for stable.

- Alex