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

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

[Jouni Malinen]

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(-)

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 },
-- 
2.7.4

[PATCH 2/4] cfg80211: Check if NAN service ID is of expected size

[Jouni Malinen]

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, cfg80211 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_NAN_FUNC_SERVICE_ID to make these NLA_UNSPEC and to make sure
minimum NL80211_NAN_FUNC_SERVICE_ID_LEN bytes are received from
userspace with NL80211_NAN_FUNC_SERVICE_ID.

Fixes: a442b761b24 ("cfg80211: add add_nan_func / del_nan_func")
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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 364291c..dcfeae0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -519,7 +519,7 @@ nl80211_bss_select_policy[NL80211_BSS_SELECT_ATTR_MAX + 1] = {
 static const struct nla_policy
 nl80211_nan_func_policy[NL80211_NAN_FUNC_ATTR_MAX + 1] = {
 	[NL80211_NAN_FUNC_TYPE] = { .type = NLA_U8 },
-	[NL80211_NAN_FUNC_SERVICE_ID] = { .type = NLA_BINARY,
+	[NL80211_NAN_FUNC_SERVICE_ID] = {
 				    .len = NL80211_NAN_FUNC_SERVICE_ID_LEN },
 	[NL80211_NAN_FUNC_PUBLISH_TYPE] = { .type = NLA_U8 },
 	[NL80211_NAN_FUNC_PUBLISH_BCAST] = { .type = NLA_FLAG },
-- 
2.7.4

[PATCH 3/4] cfg80211: Define nla_policy for NL80211_ATTR_LOCAL_MESH_POWER_MODE

[Jouni Malinen]

From: Srinivas Dasari <dasaris@qti.qualcomm.com>

Buffer overread may happen as nl80211_set_station() reads 4 bytes
from the attribute NL80211_ATTR_LOCAL_MESH_POWER_MODE without
validating the size of data received when userspace sends less
than 4 bytes of data with NL80211_ATTR_LOCAL_MESH_POWER_MODE.
Define nla_policy for NL80211_ATTR_LOCAL_MESH_POWER_MODE to avoid
the buffer overread.

Fixes: 3b1c5a5307f ("{cfg,nl}80211: mesh power mode primitives and userspace access")
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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index dcfeae0..0a63b95 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -347,6 +347,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
 	[NL80211_ATTR_SCAN_FLAGS] = { .type = NLA_U32 },
 	[NL80211_ATTR_P2P_CTWINDOW] = { .type = NLA_U8 },
 	[NL80211_ATTR_P2P_OPPPS] = { .type = NLA_U8 },
+	[NL80211_ATTR_LOCAL_MESH_POWER_MODE] = {. type = NLA_U32 },
 	[NL80211_ATTR_ACL_POLICY] = {. type = NLA_U32 },
 	[NL80211_ATTR_MAC_ADDRS] = { .type = NLA_NESTED },
 	[NL80211_ATTR_STA_CAPABILITY] = { .type = NLA_U16 },
-- 
2.7.4

[PATCH 4/4] cfg80211: Validate frequencies nested in NL80211_ATTR_SCAN_FREQUENCIES

[Jouni Malinen]

From: Srinivas Dasari <dasaris@qti.qualcomm.com>

validate_scan_freqs() retrieves frequencies from attributes
nested in the attribute NL80211_ATTR_SCAN_FREQUENCIES with
nla_get_u32(), which reads 4 bytes from each attribute
without validating the size of data received. Attributes
nested in NL80211_ATTR_SCAN_FREQUENCIES don't have an nla policy.

Validate size of each attribute before parsing to avoid potential buffer
overread.

Fixes: 2a519311926 ("cfg80211/nl80211: scanning (and mac80211 update to use it)")
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, 3 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 0a63b95..740d3c1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6469,6 +6469,9 @@ static int validate_scan_freqs(struct nlattr *freqs)
 	struct nlattr *attr1, *attr2;
 	int n_channels = 0, tmp1, tmp2;
 
+	nla_for_each_nested(attr1, freqs, tmp1)
+		if (nla_len(attr1) != sizeof(u32))
+			return 0;
 	nla_for_each_nested(attr1, freqs, tmp1) {
 		n_channels++;
 		/*
-- 
2.7.4

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

[Johannes Berg]

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

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

[Jouni Malinen]

On Fri, Jul 07, 2017 at 11:27:56AM +0200, Johannes Berg wrote:
> All applied, thanks.
>=20
> 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.

--=20
Jouni Malinen                                            PGP id EFC895FA=

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

[Johannes Berg]

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