* [PATCH] net/ieee802154: fix uninit value bug in dgram_sendmsg
@ 2022-08-22 7:19 Haimin Zhang
2022-08-23 8:40 ` Stefan Schmidt
0 siblings, 1 reply; 6+ messages in thread
From: Haimin Zhang @ 2022-08-22 7:19 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.
This length is specified by msg->msg_namelen. Function
ieee802154_addr_from_sa is called by dgram_sendmsg, which use
msg->msg_name as struct sockaddr_ieee802154* and read it, that will
eventually lead to uninit value read. So we should check the length of
msg->msg_name is not less than sizeof(struct sockaddr_ieee802154)
before entering the ieee802154_addr_from_sa.
Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
---
net/ieee802154/socket.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 718fb77bb..efbe08590 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -655,6 +655,10 @@ static int dgram_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
if (msg->msg_name) {
DECLARE_SOCKADDR(struct sockaddr_ieee802154*,
daddr, msg->msg_name);
+ if (msg->msg_namelen < sizeof(*daddr)) {
+ err = -EINVAL;
+ goto out_skb;
+ }
ieee802154_addr_from_sa(&dst_addr, &daddr->addr);
} else {
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net/ieee802154: fix uninit value bug in dgram_sendmsg
2022-08-22 7:19 [PATCH] net/ieee802154: fix uninit value bug in dgram_sendmsg Haimin Zhang
@ 2022-08-23 8:40 ` Stefan Schmidt
2022-08-23 12:22 ` Alexander Aring
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Schmidt @ 2022-08-23 8:40 UTC (permalink / raw)
To: Haimin Zhang, alex.aring, davem, edumazet, kuba, pabeni,
linux-wpan, netdev, linux-kernel
Cc: Haimin Zhang
Hello.
On 22.08.22 09:19, Haimin Zhang 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.
>
> This length is specified by msg->msg_namelen. Function
> ieee802154_addr_from_sa is called by dgram_sendmsg, which use
> msg->msg_name as struct sockaddr_ieee802154* and read it, that will
> eventually lead to uninit value read. So we should check the length of
> msg->msg_name is not less than sizeof(struct sockaddr_ieee802154)
> before entering the ieee802154_addr_from_sa.
>
> Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
This patch has been applied to the wpan tree and will be
part of the next pull request to net. Thanks!
Btw, I got a warning from the checkpatch script that your author and SOB
email addresses do not match. Might be a good idea to fix this.
If you are having trouble to send patches through the company mail
server there is always the option to use the gmail address for sending
the mail and an internal From: header in the patch to fix up the author.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/ieee802154: fix uninit value bug in dgram_sendmsg
2022-08-23 8:40 ` Stefan Schmidt
@ 2022-08-23 12:22 ` Alexander Aring
[not found] ` <CAB2z9exhnzte0rpT9t6=VpFCm9x+zZdmr01UHFxqvYy8y9ifag@mail.gmail.com>
2022-08-29 9:08 ` Stefan Schmidt
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Aring @ 2022-08-23 12:22 UTC (permalink / raw)
To: Stefan Schmidt
Cc: Haimin Zhang, Alexander Aring, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-wpan - ML, Network Development,
Linux Kernel Mailing List, Haimin Zhang
Hi,
On Tue, Aug 23, 2022 at 5:42 AM Stefan Schmidt
<stefan@datenfreihafen.org> wrote:
>
> Hello.
>
> On 22.08.22 09:19, Haimin Zhang 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.
> >
> > This length is specified by msg->msg_namelen. Function
> > ieee802154_addr_from_sa is called by dgram_sendmsg, which use
> > msg->msg_name as struct sockaddr_ieee802154* and read it, that will
> > eventually lead to uninit value read. So we should check the length of
> > msg->msg_name is not less than sizeof(struct sockaddr_ieee802154)
> > before entering the ieee802154_addr_from_sa.
> >
> > Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
>
>
> This patch has been applied to the wpan tree and will be
> part of the next pull request to net. Thanks!
For me this patch is buggy or at least it is questionable how to deal
with the size of ieee802154_addr_sa here.
There should be a helper to calculate the size which depends on the
addr_type field. It is not required to send the last 6 bytes if
addr_type is IEEE802154_ADDR_SHORT.
Nitpick is that we should check in the beginning of that function.
- Alex
^ permalink raw reply [flat|nested] 6+ messages in thread[parent not found: <CAB2z9exhnzte0rpT9t6=VpFCm9x+zZdmr01UHFxqvYy8y9ifag@mail.gmail.com>]
* Re: [PATCH] net/ieee802154: fix uninit value bug in dgram_sendmsg
[not found] ` <CAB2z9exhnzte0rpT9t6=VpFCm9x+zZdmr01UHFxqvYy8y9ifag@mail.gmail.com>
@ 2022-08-24 12:38 ` Alexander Aring
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Aring @ 2022-08-24 12:38 UTC (permalink / raw)
To: zhang haiming
Cc: linux-wpan - ML, Network Development, Linux Kernel Mailing List
Hi,
cc mailing lists again.
On Wed, Aug 24, 2022 at 7:55 AM zhang haiming <tcs.kernel@gmail.com> wrote:
>
> If msg_namelen is too small like 1, the addr_type field will be
> unexpected. Meanwhile, check msg_namelen < sizeof(*daddr) is
Then check if space for addr_type is available, if not -EINVAL. If
addr_type available, evaluate it, if it's unknown -EINVAL, the minimum
length differs here if it's known.
> necessary and enough as dgram_bind and dgram_connect did.
>
you probably found similar issues.
It is a nitpick and I see that the current behaviour is not correct here.
- Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net/ieee802154: fix uninit value bug in dgram_sendmsg
2022-08-23 12:22 ` Alexander Aring
[not found] ` <CAB2z9exhnzte0rpT9t6=VpFCm9x+zZdmr01UHFxqvYy8y9ifag@mail.gmail.com>
@ 2022-08-29 9:08 ` Stefan Schmidt
2022-08-30 7:04 ` zhang haiming
1 sibling, 1 reply; 6+ messages in thread
From: Stefan Schmidt @ 2022-08-29 9:08 UTC (permalink / raw)
To: Alexander Aring
Cc: Haimin Zhang, Alexander Aring, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-wpan - ML, Network Development,
Linux Kernel Mailing List, Haimin Zhang
Hello Alex.
On 23.08.22 14:22, Alexander Aring wrote:
> Hi,
>
> On Tue, Aug 23, 2022 at 5:42 AM Stefan Schmidt
> <stefan@datenfreihafen.org> wrote:
>>
>> Hello.
>>
>> On 22.08.22 09:19, Haimin Zhang 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.
>>>
>>> This length is specified by msg->msg_namelen. Function
>>> ieee802154_addr_from_sa is called by dgram_sendmsg, which use
>>> msg->msg_name as struct sockaddr_ieee802154* and read it, that will
>>> eventually lead to uninit value read. So we should check the length of
>>> msg->msg_name is not less than sizeof(struct sockaddr_ieee802154)
>>> before entering the ieee802154_addr_from_sa.
>>>
>>> Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
>>
>>
>> This patch has been applied to the wpan tree and will be
>> part of the next pull request to net. Thanks!
>
> For me this patch is buggy or at least it is questionable how to deal
> with the size of ieee802154_addr_sa here.
You are right. I completely missed this. Thanks for spotting!
> There should be a helper to calculate the size which depends on the
> addr_type field. It is not required to send the last 6 bytes if
> addr_type is IEEE802154_ADDR_SHORT.
> Nitpick is that we should check in the beginning of that function.
Haimin, in ieee802154 we could have two different sizes for
ieee802154_addr_sa depending on the addr_type. We have short and
extended addresses.
Could you please rework this patch to take this into account as Alex
suggested?
I reverted your original patch from my tree.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net/ieee802154: fix uninit value bug in dgram_sendmsg
2022-08-29 9:08 ` Stefan Schmidt
@ 2022-08-30 7:04 ` zhang haiming
0 siblings, 0 replies; 6+ messages in thread
From: zhang haiming @ 2022-08-30 7:04 UTC (permalink / raw)
To: Stefan Schmidt
Cc: Alexander Aring, Alexander Aring, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-wpan - ML, Network Development,
Linux Kernel Mailing List, Haimin Zhang
Thanks to all.
I have sent patch v2 to fix this.
On Mon, Aug 29, 2022 at 5:08 PM Stefan Schmidt
<stefan@datenfreihafen.org> wrote:
>
>
> Hello Alex.
>
> On 23.08.22 14:22, Alexander Aring wrote:
> > Hi,
> >
> > On Tue, Aug 23, 2022 at 5:42 AM Stefan Schmidt
> > <stefan@datenfreihafen.org> wrote:
> >>
> >> Hello.
> >>
> >> On 22.08.22 09:19, Haimin Zhang 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.
> >>>
> >>> This length is specified by msg->msg_namelen. Function
> >>> ieee802154_addr_from_sa is called by dgram_sendmsg, which use
> >>> msg->msg_name as struct sockaddr_ieee802154* and read it, that will
> >>> eventually lead to uninit value read. So we should check the length of
> >>> msg->msg_name is not less than sizeof(struct sockaddr_ieee802154)
> >>> before entering the ieee802154_addr_from_sa.
> >>>
> >>> Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
> >>
> >>
> >> This patch has been applied to the wpan tree and will be
> >> part of the next pull request to net. Thanks!
> >
> > For me this patch is buggy or at least it is questionable how to deal
> > with the size of ieee802154_addr_sa here.
>
> You are right. I completely missed this. Thanks for spotting!
>
> > There should be a helper to calculate the size which depends on the
> > addr_type field. It is not required to send the last 6 bytes if
> > addr_type is IEEE802154_ADDR_SHORT.
> > Nitpick is that we should check in the beginning of that function.
>
> Haimin, in ieee802154 we could have two different sizes for
> ieee802154_addr_sa depending on the addr_type. We have short and
> extended addresses.
>
> Could you please rework this patch to take this into account as Alex
> suggested?
>
> I reverted your original patch from my tree.
>
> regards
> Stefan Schmidt
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-30 7:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-22 7:19 [PATCH] net/ieee802154: fix uninit value bug in dgram_sendmsg Haimin Zhang
2022-08-23 8:40 ` Stefan Schmidt
2022-08-23 12:22 ` Alexander Aring
[not found] ` <CAB2z9exhnzte0rpT9t6=VpFCm9x+zZdmr01UHFxqvYy8y9ifag@mail.gmail.com>
2022-08-24 12:38 ` Alexander Aring
2022-08-29 9:08 ` Stefan Schmidt
2022-08-30 7:04 ` zhang haiming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).