diff mbox

[1/4] cfg80211: Check if PMKID attribute is of expected size

Message ID 1499381022-5389-1-git-send-email-jouni@qca.qualcomm.com (mailing list archive)
State Accepted
Delegated to: Johannes Berg
Headers show

Commit Message

Jouni Malinen July 6, 2017, 10:43 p.m. UTC 
From: Srinivas Dasari <dasaris@qti.qualcomm.com>

nla policy checks for only maximum length of the attribute data
when the attribute type is NLA_BINARY. If userspace sends less
data than specified, the wireless drivers may access illegal
memory. When type is NLA_UNSPEC, nla policy check ensures that
userspace sends minimum specified length number of bytes.

Remove type assignment to NLA_BINARY from nla_policy of
NL80211_ATTR_PMKID to make this NLA_UNSPEC and to make sure minimum
WLAN_PMKID_LEN bytes are received from userspace with
NL80211_ATTR_PMKID.

Fixes: 67fbb16be69d ("nl80211: PMKSA caching support")
Cc: stable@vger.kernel.org
Signed-off-by: Srinivas Dasari <dasaris@qti.qualcomm.com>
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 net/wireless/nl80211.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Johannes Berg July 7, 2017, 9:27 a.m. UTC | #1 
All applied, thanks.

How did you find these? Some of them go way back, after all, and I
don't think even coverity flagged them?

johannes
Jouni Malinen July 7, 2017, 9:36 a.m. UTC | #2 
On Fri, Jul 07, 2017 at 11:27:56AM +0200, Johannes Berg wrote:
> All applied, thanks.
> 
> How did you find these? Some of them go way back, after all, and I
> don't think even coverity flagged them?

Through manual review of all the nl80211 attributes.. There have been
somewhat similar issues flagged as security issues in various kernel
components recently and that has triggered more scrutiny for the kernel
interfaces.
Johannes Berg July 7, 2017, 11:41 a.m. UTC | #3 
On Fri, 2017-07-07 at 09:36 +0000, Jouni Malinen wrote:
> On Fri, Jul 07, 2017 at 11:27:56AM +0200, Johannes Berg wrote:
> > All applied, thanks.
> > 
> > How did you find these? Some of them go way back, after all, and I
> > don't think even coverity flagged them?
> 
> Through manual review of all the nl80211 attributes.. There have been
> somewhat similar issues flagged as security issues in various kernel
> components recently and that has triggered more scrutiny for the
> kernel interfaces.

Cool, thanks for that. I almost thought (hoped?) you had a (new) tool
to detect this :)

johannes
diff mbox

Patch

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5487cd7..364291c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -291,8 +291,7 @@  static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 	[NL80211_ATTR_WPA_VERSIONS] = { .type = NLA_U32 },
 	[NL80211_ATTR_PID] = { .type = NLA_U32 },
 	[NL80211_ATTR_4ADDR] = { .type = NLA_U8 },
-	[NL80211_ATTR_PMKID] = { .type = NLA_BINARY,
-				 .len = WLAN_PMKID_LEN },
+	[NL80211_ATTR_PMKID] = { .len = WLAN_PMKID_LEN },
 	[NL80211_ATTR_DURATION] = { .type = NLA_U32 },
 	[NL80211_ATTR_COOKIE] = { .type = NLA_U64 },
 	[NL80211_ATTR_TX_RATES] = { .type = NLA_NESTED },