78 lines
2.9 KiB
Diff
78 lines
2.9 KiB
Diff
From: Johannes Berg <johannes.berg@intel.com>
|
|
Date: Mon, 30 Aug 2010 10:24:54 +0000 (+0200)
|
|
Subject: wireless extensions: fix kernel heap content leak
|
|
X-Git-Tag: master-2010-08-30
|
|
X-Git-Url: http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Flinville%2Fwireless-2.6.git;a=commitdiff_plain;h=42da2f948d949efd0111309f5827bf0298bcc9a4
|
|
|
|
wireless extensions: fix kernel heap content leak
|
|
|
|
Wireless extensions have an unfortunate, undocumented
|
|
requirement which requires drivers to always fill
|
|
iwp->length when returning a successful status. When
|
|
a driver doesn't do this, it leads to a kernel heap
|
|
content leak when userspace offers a larger buffer
|
|
than would have been necessary.
|
|
|
|
Arguably, this is a driver bug, as it should, if it
|
|
returns 0, fill iwp->length, even if it separately
|
|
indicated that the buffer contents was not valid.
|
|
|
|
However, we can also at least avoid the memory content
|
|
leak if the driver doesn't do this by setting the iwp
|
|
length to max_tokens, which then reflects how big the
|
|
buffer is that the driver may fill, regardless of how
|
|
big the userspace buffer is.
|
|
|
|
To illustrate the point, this patch also fixes a
|
|
corresponding cfg80211 bug (since this requirement
|
|
isn't documented nor was ever pointed out by anyone
|
|
during code review, I don't trust all drivers nor
|
|
all cfg80211 handlers to implement it correctly).
|
|
|
|
Cc: stable@kernel.org [all the way back]
|
|
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
|
|
Signed-off-by: John W. Linville <linville@tuxdriver.com>
|
|
---
|
|
|
|
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
|
|
index bb5e0a5..7e5c3a4 100644
|
|
--- a/net/wireless/wext-compat.c
|
|
+++ b/net/wireless/wext-compat.c
|
|
@@ -1420,6 +1420,9 @@ int cfg80211_wext_giwessid(struct net_device *dev,
|
|
{
|
|
struct wireless_dev *wdev = dev->ieee80211_ptr;
|
|
|
|
+ data->flags = 0;
|
|
+ data->length = 0;
|
|
+
|
|
switch (wdev->iftype) {
|
|
case NL80211_IFTYPE_ADHOC:
|
|
return cfg80211_ibss_wext_giwessid(dev, info, data, ssid);
|
|
diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
|
|
index 0ef17bc..8f5116f 100644
|
|
--- a/net/wireless/wext-core.c
|
|
+++ b/net/wireless/wext-core.c
|
|
@@ -782,6 +782,22 @@ static int ioctl_standard_iw_point(struct iw_point *iwp, unsigned int cmd,
|
|
}
|
|
}
|
|
|
|
+ if (IW_IS_GET(cmd) && !(descr->flags & IW_DESCR_FLAG_NOMAX)) {
|
|
+ /*
|
|
+ * If this is a GET, but not NOMAX, it means that the extra
|
|
+ * data is not bounded by userspace, but by max_tokens. Thus
|
|
+ * set the length to max_tokens. This matches the extra data
|
|
+ * allocation.
|
|
+ * The driver should fill it with the number of tokens it
|
|
+ * provided, and it may check iwp->length rather than having
|
|
+ * knowledge of max_tokens. If the driver doesn't change the
|
|
+ * iwp->length, this ioctl just copies back max_token tokens
|
|
+ * filled with zeroes. Hopefully the driver isn't claiming
|
|
+ * them to be valid data.
|
|
+ */
|
|
+ iwp->length = descr->max_tokens;
|
|
+ }
|
|
+
|
|
err = handler(dev, info, (union iwreq_data *) iwp, extra);
|
|
|
|
iwp->length += essid_compat;
|