Skip to contentSkip to conversation

cfg80211: Fix changing regulatory from userspace

PATCH In review Timo Lindhorst 4 participants 8 messages Nov 23, 2011 – Nov 29, 2011 802.11 (including cfg80211/NL80211)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cfg80211: Fix changing regulatory from userspace

The commit de3584bd62d87b4c250129fbc46ca52c80330add - "cfg80211: fix regulatory NULL dereference" prevents the regulatory domain from being changed by user space. A wiphy is only present if the request comes from driver or is set by country IE, thus check only those cases.

---
 net/wireless/reg.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
net/wireless/reg.c +3 -1
index 76b35df..c9588ae 100644
@@ -2076,7 +2076,9 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	}
 
 	request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
-	if (!request_wiphy) {
+	if (((last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER) ||
+	     (last_request->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)) &&
+	    !request_wiphy) {
 		reg_set_request_processed();
 		return -ENODEV;
 	}
Signature · 1 line
1.7.7.3
#1 lore raw
Quoted 7 lines · Timo Lindhorst

The commit de3584bd62d87b4c250129fbc46ca52c80330add - "cfg80211: fix regulatory NULL dereference" prevents the regulatory domain from being changed by user space. A wiphy is only present if the request comes from driver or is set by country IE, thus check only those cases.

Yup but at this point I'd prefer we revert the original patch instead given that the patch also had some other short comings. John is it too late?

Luis

#2 lore raw
Quoted 7 lines · Luis R. Rodriguez
Quoted 6 lines · Timo Lindhorst

The commit de3584bd62d87b4c250129fbc46ca52c80330add - "cfg80211: fix regulatory NULL dereference" prevents the regulatory domain from being changed by user space. A wiphy is only present if the request comes from driver or is set by country IE, thus check only those cases.

Quoted 5 lines
Quoted 1 line

Yup but at this point I'd prefer we revert the original patch instead given that the patch also had some other short comings. John is it too late?

No, it isn't too late to revert it. But can we have a new fix soon?

John

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.
#3 lore raw
Quoted 8 lines · John W. Linville
Quoted 7 lines · Luis R. Rodriguez
Quoted 6 lines · Timo Lindhorst

The commit de3584bd62d87b4c250129fbc46ca52c80330add - "cfg80211: fix regulatory NULL dereference" prevents the regulatory domain from being changed by user space. A wiphy is only present if the request comes from driver or is set by country IE, thus check only those cases.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Yup but at this point I'd prefer we revert the original patch instead given that the patch also had some other short comings. John is it too late?

No, it isn't too late to revert it. But can we have a new fix soon?

Hmmm...actually, the original patch was sent in the batch yesterday. And it was Cc: stable. It would be a lot less confusing to merge a correct fix on top (and Cc: stable on that too). What is wrong with this one?

John

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.
#4 lore raw
Quoted 15 lines · John W. Linville
Quoted 14 lines · John W. Linville
Quoted 12 lines · Luis R. Rodriguez
Quoted 7 lines · Timo Lindhorst

The commit de3584bd62d87b4c250129fbc46ca52c80330add - "cfg80211: fix regulatory NULL dereference" prevents the regulatory domain from being changed by user space. A wiphy is only present if the request comes from driver or is set by country IE, thus check only those cases.

Yup but at this point I'd prefer we revert the original patch instead given that the patch also had some other short comings. John is it too late?

Quoted 6 lines
Quoted 1 line

No, it isn't too late to revert it.  But can we have a new fix soon?

Hmmm...actually, the original patch was sent in the batch yesterday. And it was Cc: stable.  It would be a lot less confusing to merge a correct fix on top (and Cc: stable on that too).  What is wrong with this one?

This one is a correct fix to one of the issues present in the original patch, the other issue is the original patch overlooks the fact that last_request would be left with stale data. Let me post my fixes as RFCs on top of Johannes's original patch to clarify. Unfortunately this doesn't have much testing but I'll post for review at least.

Luis

#5 lore raw
Quoted 24 lines · Luis R. Rodriguez
Quoted 15 lines · John W. Linville
Quoted 14 lines · John W. Linville
Quoted 12 lines · Luis R. Rodriguez
Quoted 7 lines · Timo Lindhorst

The commit de3584bd62d87b4c250129fbc46ca52c80330add - "cfg80211: fix regulatory NULL dereference" prevents the regulatory domain from being changed by user space. A wiphy is only present if the request comes from driver or is set by country IE, thus check only those cases.

Yup but at this point I'd prefer we revert the original patch instead given that the patch also had some other short comings. John is it too late?

Quoted 6 lines
Quoted 1 line

No, it isn't too late to revert it.  But can we have a new fix soon?

Hmmm...actually, the original patch was sent in the batch yesterday. And it was Cc: stable.  It would be a lot less confusing to merge a correct fix on top (and Cc: stable on that too).  What is wrong with this one?

Quoted 5 lines

This one is a correct fix to one of the issues present in the original patch, the other issue is the original patch overlooks the fact that last_request would be left with stale data. Let me post my fixes as RFCs on top of Johannes's original patch to clarify. Unfortunately this doesn't have much testing but I'll post for review at least.

Looks like the RFCs got some comments. Will there be a respin of those? Or should I just apply this one for now?

John

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.
#6 lore raw
Quoted 25 lines · John W. Linville
Quoted 24 lines · Luis R. Rodriguez
Quoted 21 lines · John W. Linville
Quoted 8 lines · John W. Linville
Quoted 7 lines · Luis R. Rodriguez
Quoted 6 lines · Timo Lindhorst

The commit de3584bd62d87b4c250129fbc46ca52c80330add - "cfg80211: fix regulatory NULL dereference" prevents the regulatory domain from being changed by user space. A wiphy is only present if the request comes from driver or is set by country IE, thus check only those cases.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Yup but at this point I'd prefer we revert the original patch instead given that the patch also had some other short comings. John is it too late?

No, it isn't too late to revert it.  But can we have a new fix soon?

Hmmm...actually, the original patch was sent in the batch yesterday. And it was Cc: stable.  It would be a lot less confusing to merge a correct fix on top (and Cc: stable on that too).  What is wrong with this one?

Quoted 8 lines
Quoted 5 lines

This one is a correct fix to one of the issues present in the original patch, the other issue is the original patch overlooks the fact that last_request would be left with stale data. Let me post my fixes as RFCs on top of Johannes's original patch to clarify. Unfortunately this doesn't have much testing but I'll post for review at least.

Looks like the RFCs got some comments.  Will there be a respin of those?  Or should I just apply this one for now?

John, I'll will send a respin after I dissect what Tomas mentioned. I have one other patch added to my series also that Rajkumar Manoharan <[email protected]> sent to me that I will be rolling into my series. If we are pressed my recommendation is to revert Johannes' patch for now, but if you do, please let me know so I can amend my patches to address that bit.

I'm working on this right now.

Luis

maintainer
#7 lore raw
Quoted 6 lines · Luis R. Rodriguez

John, I'll will send a respin after I dissect what Tomas mentioned. I have one other patch added to my series also that Rajkumar Manoharan <[email protected]> sent to me that I will be rolling into my series. If we are pressed my recommendation is to revert Johannes' patch for now, but if you do, please let me know so I can amend my patches to address that bit.

I really don't want to revert that since it fixes a NULL pointer dereference that you can probably trigger fairly easily from userspace with not much more than well-timed unplug and lots of disk activity.

Why can't you just work on the fix first and then do all those improvement patches?

johannes