* [PATCH V2] net/ieee802154: fix uninit value bug in dgram_sendmsg
@ 2022-08-30 7:03 Haimin Zhang
2022-08-30 11:37 ` kernel test robot
2022-09-08 2:02 ` Alexander Aring
0 siblings, 2 replies; 4+ messages in thread
From: Haimin Zhang @ 2022-08-30 7:03 UTC (permalink / raw)
To: alex.aring, stefan, davem, edumazet, kuba, pabeni, linux-wpan,
netdev, linux-kernel
Cc: Haimin Zhang
There is uninit value bug in dgram_sendmsg function in
net/ieee802154/socket.c when the length of valid data pointed by the
msg->msg_name isn't verified.
We should check the msg_namelen is not less than struct
sockaddr_ieee802154 when addr_type is SHORT before calling
ieee802154_addr_from_sa. So we define IEEE802154_MIN_NAMELEN.
And in function ieee802154_addr_from_sa, when
addr_type is LONG, we check msg_namelen is not less than
sizeof(struct sockaddr_ieee802154). Meanwhile we check in the
beginning of function dgram_sendmsg.
Also fixed in raw_bind, dgram_bind, dgram_connect.
Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
---
include/net/ieee802154_netdev.h | 9 ++++--
net/ieee802154/socket.c | 52 ++++++++++++++++++++-------------
2 files changed, 39 insertions(+), 22 deletions(-)
diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index d0d188c32..e1dc3fb02 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -165,8 +165,8 @@ static inline void ieee802154_devaddr_to_raw(void *raw, __le64 addr)
memcpy(raw, &temp, IEEE802154_ADDR_LEN);
}
-static inline void ieee802154_addr_from_sa(struct ieee802154_addr *a,
- const struct ieee802154_addr_sa *sa)
+static inline int ieee802154_addr_from_sa(struct ieee802154_addr *a,
+ const struct ieee802154_addr_sa *sa, int len)
{
a->mode = sa->addr_type;
a->pan_id = cpu_to_le16(sa->pan_id);
@@ -176,9 +176,14 @@ static inline void ieee802154_addr_from_sa(struct ieee802154_addr *a,
a->short_addr = cpu_to_le16(sa->short_addr);
break;
case IEEE802154_ADDR_LONG:
+ if (len > sizeof(struct sockaddr_ieee802154))
+ return -EINVAL;
a->extended_addr = ieee802154_devaddr_from_raw(sa->hwaddr);
break;
+ default:
+ return -EINVAL;
}
+ return 0;
}
static inline void ieee802154_addr_to_sa(struct ieee802154_addr_sa *sa,
diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 718fb77bb..4452aaf68 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -27,6 +27,10 @@
#include <net/af_ieee802154.h>
#include <net/ieee802154_netdev.h>
+#define IEEE802154_MIN_NAMELEN \
+ (offsetof(struct sockaddr_ieee802154, addr) + \
+ offsetofend(struct ieee802154_addr_sa, short_addr))
+
/* Utility function for families */
static struct net_device*
ieee802154_get_dev(struct net *net, const struct ieee802154_addr *addr)
@@ -200,7 +204,7 @@ static int raw_bind(struct sock *sk, struct sockaddr *_uaddr, int len)
int err = 0;
struct net_device *dev = NULL;
- if (len < sizeof(*uaddr))
+ if (len < IEEE802154_MIN_NAMELEN)
return -EINVAL;
uaddr = (struct sockaddr_ieee802154 *)_uaddr;
@@ -209,7 +213,9 @@ static int raw_bind(struct sock *sk, struct sockaddr *_uaddr, int len)
lock_sock(sk);
- ieee802154_addr_from_sa(&addr, &uaddr->addr);
+ err = ieee802154_addr_from_sa(&addr, &uaddr->addr, len);
+ if (err < 0)
+ goto out;
dev = ieee802154_get_dev(sock_net(sk), &addr);
if (!dev) {
err = -ENODEV;
@@ -493,13 +499,15 @@ static int dgram_bind(struct sock *sk, struct sockaddr *uaddr, int len)
ro->bound = 0;
- if (len < sizeof(*addr))
+ if (len < IEEE802154_MIN_NAMELEN)
goto out;
if (addr->family != AF_IEEE802154)
goto out;
- ieee802154_addr_from_sa(&haddr, &addr->addr);
+ err = ieee802154_addr_from_sa(&haddr, &addr->addr, len);
+ if (err < 0)
+ goto out;
dev = ieee802154_get_dev(sock_net(sk), &haddr);
if (!dev) {
err = -ENODEV;
@@ -564,7 +572,7 @@ static int dgram_connect(struct sock *sk, struct sockaddr *uaddr,
struct dgram_sock *ro = dgram_sk(sk);
int err = 0;
- if (len < sizeof(*addr))
+ if (len < IEEE802154_MIN_NAMELEN)
return -EINVAL;
if (addr->family != AF_IEEE802154)
@@ -577,7 +585,9 @@ static int dgram_connect(struct sock *sk, struct sockaddr *uaddr,
goto out;
}
- ieee802154_addr_from_sa(&ro->dst_addr, &addr->addr);
+ err = ieee802154_addr_from_sa(&ro->dst_addr, &addr->addr, len);
+ if (err < 0)
+ goto out;
ro->connected = 1;
out:
@@ -612,10 +622,22 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
return -EOPNOTSUPP;
}
- if (!ro->connected && !msg->msg_name)
- return -EDESTADDRREQ;
- else if (ro->connected && msg->msg_name)
- return -EISCONN;
+ if (msg->msg_name) {
+ if (ro->connected)
+ return -EISCONN;
+ if (msg->msg_namelen < IEEE802154_MIN_NAMELEN)
+ return -EINVAL;
+ DECLARE_SOCKADDR(struct sockaddr_ieee802154*,
+ daddr, msg->msg_name);
+ err = ieee802154_addr_from_sa(&dst_addr, &daddr->addr,
+ msg->msg_namelen);
+ if (err < 0)
+ return err;
+ } else {
+ if (!ro->connected)
+ return -EDESTADDRREQ;
+ dst_addr = ro->dst_addr;
+ }
if (!ro->bound)
dev = dev_getfirstbyhwtype(sock_net(sk), ARPHRD_IEEE802154);
@@ -651,16 +673,6 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
cb = mac_cb_init(skb);
cb->type = IEEE802154_FC_TYPE_DATA;
cb->ackreq = ro->want_ack;
-
- if (msg->msg_name) {
- DECLARE_SOCKADDR(struct sockaddr_ieee802154*,
- daddr, msg->msg_name);
-
- ieee802154_addr_from_sa(&dst_addr, &daddr->addr);
- } else {
- dst_addr = ro->dst_addr;
- }
-
cb->secen = ro->secen;
cb->secen_override = ro->secen_override;
cb->seclevel = ro->seclevel;
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V2] net/ieee802154: fix uninit value bug in dgram_sendmsg
2022-08-30 7:03 Haimin Zhang
@ 2022-08-30 11:37 ` kernel test robot
2022-09-08 2:02 ` Alexander Aring
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2022-08-30 11:37 UTC (permalink / raw)
To: Haimin Zhang; +Cc: llvm, kbuild-all
Hi Haimin,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on net/master linus/master v6.0-rc3 next-20220830]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Haimin-Zhang/net-ieee802154-fix-uninit-value-bug-in-dgram_sendmsg/20220830-150500
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f97e971dbdc7c83d697fa2209fed0ea50fffa12e
config: hexagon-randconfig-r045-20220830 (https://download.01.org/0day-ci/archive/20220830/202208301926.Dbvh7eGF-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project c7df82e4693c19e3fd2e25c83eb04d9deb7b7b59)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/10679908140f660fb3391ea1d12f03365d95060f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Haimin-Zhang/net-ieee802154-fix-uninit-value-bug-in-dgram_sendmsg/20220830-150500
git checkout 10679908140f660fb3391ea1d12f03365d95060f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash net/ieee802154/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/ieee802154/socket.c:631:6: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
daddr, msg->msg_name);
^
1 warning generated.
vim +631 net/ieee802154/socket.c
608
609 static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
610 {
611 struct net_device *dev;
612 unsigned int mtu;
613 struct sk_buff *skb;
614 struct ieee802154_mac_cb *cb;
615 struct dgram_sock *ro = dgram_sk(sk);
616 struct ieee802154_addr dst_addr;
617 int hlen, tlen;
618 int err;
619
620 if (msg->msg_flags & MSG_OOB) {
621 pr_debug("msg->msg_flags = 0x%x\n", msg->msg_flags);
622 return -EOPNOTSUPP;
623 }
624
625 if (msg->msg_name) {
626 if (ro->connected)
627 return -EISCONN;
628 if (msg->msg_namelen < IEEE802154_MIN_NAMELEN)
629 return -EINVAL;
630 DECLARE_SOCKADDR(struct sockaddr_ieee802154*,
> 631 daddr, msg->msg_name);
632 err = ieee802154_addr_from_sa(&dst_addr, &daddr->addr,
633 msg->msg_namelen);
634 if (err < 0)
635 return err;
636 } else {
637 if (!ro->connected)
638 return -EDESTADDRREQ;
639 dst_addr = ro->dst_addr;
640 }
641
642 if (!ro->bound)
643 dev = dev_getfirstbyhwtype(sock_net(sk), ARPHRD_IEEE802154);
644 else
645 dev = ieee802154_get_dev(sock_net(sk), &ro->src_addr);
646
647 if (!dev) {
648 pr_debug("no dev\n");
649 err = -ENXIO;
650 goto out;
651 }
652 mtu = IEEE802154_MTU;
653 pr_debug("name = %s, mtu = %u\n", dev->name, mtu);
654
655 if (size > mtu) {
656 pr_debug("size = %zu, mtu = %u\n", size, mtu);
657 err = -EMSGSIZE;
658 goto out_dev;
659 }
660
661 hlen = LL_RESERVED_SPACE(dev);
662 tlen = dev->needed_tailroom;
663 skb = sock_alloc_send_skb(sk, hlen + tlen + size,
664 msg->msg_flags & MSG_DONTWAIT,
665 &err);
666 if (!skb)
667 goto out_dev;
668
669 skb_reserve(skb, hlen);
670
671 skb_reset_network_header(skb);
672
673 cb = mac_cb_init(skb);
674 cb->type = IEEE802154_FC_TYPE_DATA;
675 cb->ackreq = ro->want_ack;
676 cb->secen = ro->secen;
677 cb->secen_override = ro->secen_override;
678 cb->seclevel = ro->seclevel;
679 cb->seclevel_override = ro->seclevel_override;
680
681 err = wpan_dev_hard_header(skb, dev, &dst_addr,
682 ro->bound ? &ro->src_addr : NULL, size);
683 if (err < 0)
684 goto out_skb;
685
686 err = memcpy_from_msg(skb_put(skb, size), msg, size);
687 if (err < 0)
688 goto out_skb;
689
690 skb->dev = dev;
691 skb->protocol = htons(ETH_P_IEEE802154);
692
693 err = dev_queue_xmit(skb);
694 if (err > 0)
695 err = net_xmit_errno(err);
696
697 dev_put(dev);
698
699 return err ?: size;
700
701 out_skb:
702 kfree_skb(skb);
703 out_dev:
704 dev_put(dev);
705 out:
706 return err;
707 }
708
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH V2] net/ieee802154: fix uninit value bug in dgram_sendmsg
@ 2022-08-30 11:48 Haimin Zhang
0 siblings, 0 replies; 4+ messages in thread
From: Haimin Zhang @ 2022-08-30 11:48 UTC (permalink / raw)
To: alex.aring, stefan, davem, edumazet, kuba, pabeni, linux-wpan,
netdev, linux-kernel
Cc: Haimin Zhang
There is uninit value bug in dgram_sendmsg function in
net/ieee802154/socket.c when the length of valid data pointed by the
msg->msg_name isn't verified.
We should check the msg_namelen is not less than struct
sockaddr_ieee802154 when addr_type is SHORT before calling
ieee802154_addr_from_sa. So we define IEEE802154_MIN_NAMELEN.
And in function ieee802154_addr_from_sa, when
addr_type is LONG, we check msg_namelen is not less than
sizeof(struct sockaddr_ieee802154). Meanwhile we check in the
beginning of function dgram_sendmsg.
Also fixed in raw_bind, dgram_bind, dgram_connect.
Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
---
include/net/ieee802154_netdev.h | 9 ++++--
net/ieee802154/socket.c | 51 ++++++++++++++++++++-------------
2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index d0d188c32..e1dc3fb02 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -165,8 +165,8 @@ static inline void ieee802154_devaddr_to_raw(void *raw, __le64 addr)
memcpy(raw, &temp, IEEE802154_ADDR_LEN);
}
-static inline void ieee802154_addr_from_sa(struct ieee802154_addr *a,
- const struct ieee802154_addr_sa *sa)
+static inline int ieee802154_addr_from_sa(struct ieee802154_addr *a,
+ const struct ieee802154_addr_sa *sa, int len)
{
a->mode = sa->addr_type;
a->pan_id = cpu_to_le16(sa->pan_id);
@@ -176,9 +176,14 @@ static inline void ieee802154_addr_from_sa(struct ieee802154_addr *a,
a->short_addr = cpu_to_le16(sa->short_addr);
break;
case IEEE802154_ADDR_LONG:
+ if (len > sizeof(struct sockaddr_ieee802154))
+ return -EINVAL;
a->extended_addr = ieee802154_devaddr_from_raw(sa->hwaddr);
break;
+ default:
+ return -EINVAL;
}
+ return 0;
}
static inline void ieee802154_addr_to_sa(struct ieee802154_addr_sa *sa,
diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 718fb77bb..f598a0241 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -27,6 +27,10 @@
#include <net/af_ieee802154.h>
#include <net/ieee802154_netdev.h>
+#define IEEE802154_MIN_NAMELEN \
+ (offsetof(struct sockaddr_ieee802154, addr) + \
+ offsetofend(struct ieee802154_addr_sa, short_addr))
+
/* Utility function for families */
static struct net_device*
ieee802154_get_dev(struct net *net, const struct ieee802154_addr *addr)
@@ -200,7 +204,7 @@ static int raw_bind(struct sock *sk, struct sockaddr *_uaddr, int len)
int err = 0;
struct net_device *dev = NULL;
- if (len < sizeof(*uaddr))
+ if (len < IEEE802154_MIN_NAMELEN)
return -EINVAL;
uaddr = (struct sockaddr_ieee802154 *)_uaddr;
@@ -209,7 +213,9 @@ static int raw_bind(struct sock *sk, struct sockaddr *_uaddr, int len)
lock_sock(sk);
- ieee802154_addr_from_sa(&addr, &uaddr->addr);
+ err = ieee802154_addr_from_sa(&addr, &uaddr->addr, len);
+ if (err < 0)
+ goto out;
dev = ieee802154_get_dev(sock_net(sk), &addr);
if (!dev) {
err = -ENODEV;
@@ -493,13 +499,15 @@ static int dgram_bind(struct sock *sk, struct sockaddr *uaddr, int len)
ro->bound = 0;
- if (len < sizeof(*addr))
+ if (len < IEEE802154_MIN_NAMELEN)
goto out;
if (addr->family != AF_IEEE802154)
goto out;
- ieee802154_addr_from_sa(&haddr, &addr->addr);
+ err = ieee802154_addr_from_sa(&haddr, &addr->addr, len);
+ if (err < 0)
+ goto out;
dev = ieee802154_get_dev(sock_net(sk), &haddr);
if (!dev) {
err = -ENODEV;
@@ -564,7 +572,7 @@ static int dgram_connect(struct sock *sk, struct sockaddr *uaddr,
struct dgram_sock *ro = dgram_sk(sk);
int err = 0;
- if (len < sizeof(*addr))
+ if (len < IEEE802154_MIN_NAMELEN)
return -EINVAL;
if (addr->family != AF_IEEE802154)
@@ -577,7 +585,9 @@ static int dgram_connect(struct sock *sk, struct sockaddr *uaddr,
goto out;
}
- ieee802154_addr_from_sa(&ro->dst_addr, &addr->addr);
+ err = ieee802154_addr_from_sa(&ro->dst_addr, &addr->addr, len);
+ if (err < 0)
+ goto out;
ro->connected = 1;
out:
@@ -604,6 +614,7 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
struct ieee802154_mac_cb *cb;
struct dgram_sock *ro = dgram_sk(sk);
struct ieee802154_addr dst_addr;
+ DECLARE_SOCKADDR(struct sockaddr_ieee802154 *, daddr, msg->msg_name);
int hlen, tlen;
int err;
@@ -612,10 +623,20 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
return -EOPNOTSUPP;
}
- if (!ro->connected && !msg->msg_name)
- return -EDESTADDRREQ;
- else if (ro->connected && msg->msg_name)
- return -EISCONN;
+ if (msg->msg_name) {
+ if (ro->connected)
+ return -EISCONN;
+ if (msg->msg_namelen < IEEE802154_MIN_NAMELEN)
+ return -EINVAL;
+ err = ieee802154_addr_from_sa(&dst_addr, &daddr->addr,
+ msg->msg_namelen);
+ if (err < 0)
+ return err;
+ } else {
+ if (!ro->connected)
+ return -EDESTADDRREQ;
+ dst_addr = ro->dst_addr;
+ }
if (!ro->bound)
dev = dev_getfirstbyhwtype(sock_net(sk), ARPHRD_IEEE802154);
@@ -651,16 +672,6 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
cb = mac_cb_init(skb);
cb->type = IEEE802154_FC_TYPE_DATA;
cb->ackreq = ro->want_ack;
-
- if (msg->msg_name) {
- DECLARE_SOCKADDR(struct sockaddr_ieee802154*,
- daddr, msg->msg_name);
-
- ieee802154_addr_from_sa(&dst_addr, &daddr->addr);
- } else {
- dst_addr = ro->dst_addr;
- }
-
cb->secen = ro->secen;
cb->secen_override = ro->secen_override;
cb->seclevel = ro->seclevel;
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V2] net/ieee802154: fix uninit value bug in dgram_sendmsg
2022-08-30 7:03 Haimin Zhang
2022-08-30 11:37 ` kernel test robot
@ 2022-09-08 2:02 ` Alexander Aring
1 sibling, 0 replies; 4+ messages in thread
From: Alexander Aring @ 2022-09-08 2:02 UTC (permalink / raw)
To: Haimin Zhang
Cc: Alexander Aring, Stefan Schmidt, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-wpan - ML, Network Development,
Linux Kernel Mailing List, Haimin Zhang
Hi,
On Tue, Aug 30, 2022 at 3:03 AM Haimin Zhang <tcs.kernel@gmail.com> wrote:
>
> There is uninit value bug in dgram_sendmsg function in
> net/ieee802154/socket.c when the length of valid data pointed by the
> msg->msg_name isn't verified.
>
> We should check the msg_namelen is not less than struct
> sockaddr_ieee802154 when addr_type is SHORT before calling
> ieee802154_addr_from_sa. So we define IEEE802154_MIN_NAMELEN.
> And in function ieee802154_addr_from_sa, when
> addr_type is LONG, we check msg_namelen is not less than
> sizeof(struct sockaddr_ieee802154). Meanwhile we check in the
> beginning of function dgram_sendmsg.
>
There exists also an IEEE802154_ADDR_NONE addr_type.
We need to first check that space is there to evaluate the addr_type.
If it's NONE, ignore hwaddr or short address. If it's SHORT or hwaddr
check if they have space for it, if it's something completely
different return -EINVAL.
There are still missing bits and I would recommend introducing a
helper function to do this "kind" of more complex check. This patch
spreads different checks around by checking on IEEE802154_MIN_NAMELEN
(which isn't correct, because NONE) and then requires another check by
calling ieee802154_addr_from_sa() and checking the return code.
- Alex
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-08 2:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-30 11:48 [PATCH V2] net/ieee802154: fix uninit value bug in dgram_sendmsg Haimin Zhang
-- strict thread matches above, loose matches on Subject: below --
2022-08-30 7:03 Haimin Zhang
2022-08-30 11:37 ` kernel test robot
2022-09-08 2:02 ` Alexander Aring
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.