lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <202209240905.F5654D7A5@keescook> Date: Sat, 24 Sep 2022 09:26:01 -0700 From: Kees Cook <keescook@...omium.org> To: Hawkins Jiawei <yin31149@...il.com> Cc: 18801353760@....com, davem@...emloft.net, edumazet@...gle.com, johannes@...solutions.net, kuba@...nel.org, linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org, netdev@...r.kernel.org, pabeni@...hat.com, sfr@...b.auug.org.au, syzbot+473754e5af963cf014cf@...kaller.appspotmail.com, syzkaller-bugs@...glegroups.com Subject: Re: [PATCH] Add linux-next specific files for 20220923 On Sat, Sep 24, 2022 at 11:55:14PM +0800, Hawkins Jiawei wrote: > > And as for the value of offsetof in calculating **offset**, > > I wonder if we can use the macro defined in > > include/linux/wireless.h as below, which makes code simplier: > > #define IW_EV_COMPAT_LCP_LEN offsetof(struct __compat_iw_event, pointer) Ah yes, that would be good. > According to above code, it seems that kernel will saves enough memory > (hdr_len + extra_len bytes) for payload structure in > nla_reserve()(Please correct me if I am wrong), pointed by compat_event. > So I wonder if we can use unsafe_memcpy(), to avoid unnecessary > memcpy() check as below, which seems more simple: I'd rather this was properly resolved with the creation of a real flexible array so that when bounds tracking gets improved in the future, the compiler can reason about it better. And, I think, it makes the code more readable: diff --git a/include/linux/wireless.h b/include/linux/wireless.h index 2d1b54556eff..e0b4b46da63f 100644 --- a/include/linux/wireless.h +++ b/include/linux/wireless.h @@ -26,7 +26,10 @@ struct compat_iw_point { struct __compat_iw_event { __u16 len; /* Real length of this stuff */ __u16 cmd; /* Wireless IOCTL */ - compat_caddr_t pointer; + union { + compat_caddr_t pointer; + DECLARE_FLEX_ARRAY(__u8, ptr_bytes); + }; }; #define IW_EV_COMPAT_LCP_LEN offsetof(struct __compat_iw_event, pointer) #define IW_EV_COMPAT_POINT_OFF offsetof(struct compat_iw_point, length) diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c index 76a80a41615b..6079c8f4b634 100644 --- a/net/wireless/wext-core.c +++ b/net/wireless/wext-core.c @@ -468,6 +468,7 @@ void wireless_send_event(struct net_device * dev, struct __compat_iw_event *compat_event; struct compat_iw_point compat_wrqu; struct sk_buff *compskb; + int ptr_len; #endif /* @@ -582,6 +583,7 @@ void wireless_send_event(struct net_device * dev, nlmsg_end(skb, nlh); #ifdef CONFIG_COMPAT hdr_len = compat_event_type_size[descr->header_type]; + ptr_len = hdr_len - IW_EV_COMPAT_LCP_LEN; event_len = hdr_len + extra_len; compskb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC); @@ -612,16 +614,15 @@ void wireless_send_event(struct net_device * dev, if (descr->header_type == IW_HEADER_TYPE_POINT) { compat_wrqu.length = wrqu->data.length; compat_wrqu.flags = wrqu->data.flags; - memcpy(&compat_event->pointer, + memcpy(compat_event->ptr_bytes, ((char *) &compat_wrqu) + IW_EV_COMPAT_POINT_OFF, - hdr_len - IW_EV_COMPAT_LCP_LEN); + ptr_len); if (extra_len) - memcpy(((char *) compat_event) + hdr_len, + memcpy(compat_event->ptr_bytes + ptr_len, extra, extra_len); } else { /* extra_len must be zero, so no if (extra) needed */ - memcpy(&compat_event->pointer, wrqu, - hdr_len - IW_EV_COMPAT_LCP_LEN); + memcpy(compat_event->ptr_bytes, wrqu, ptr_len); } nlmsg_end(compskb, nlh); -- Kees Cook
Powered by blists - more mailing lists