[PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Masakazu Mokuno]

Some implementations of the hidden SSID APs emit beacons which have the zero
length SSID information element instead of SSID padded by null (\0) characters.
If the firmware of the PS3 wireless hardware meets these beacons, it abandons parsing
IEs. Thus guest OSes get the invalid scan information for the AP.

To work around this, ignore these scan informations from the list.

Signed-off-by: Masakazu Mokuno <mokuno@sm.sony.co.jp>
---

This patch was submitted for 2.6.24.3 by Geert on Mar 10.  But I have not
submitted for 2.6.25 yet.  Please apply for 2.6.25.


 drivers/net/ps3_gelic_wireless.c |   27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

--- a/drivers/net/ps3_gelic_wireless.c
+++ b/drivers/net/ps3_gelic_wireless.c
@@ -1644,13 +1644,24 @@ static void gelic_wl_scan_complete_event
 	}
 
 	/* put them in the newtork_list */
-	scan_info = wl->buf;
-	scan_info_size = 0;
-	i = 0;
-	while (scan_info_size < data_len) {
+	for (i = 0, scan_info_size = 0, scan_info = wl->buf;
+	     scan_info_size < data_len;
+	     i++, scan_info_size += be16_to_cpu(scan_info->size),
+	     scan_info = (void *)scan_info + be16_to_cpu(scan_info->size)) {
 		pr_debug("%s:size=%d bssid=%s scan_info=%p\n", __func__,
 			 be16_to_cpu(scan_info->size),
 			 print_mac(mac, &scan_info->bssid[2]), scan_info);
+
+		/*
+		 * The wireless firmware may return invalid channel 0 and/or
+		 * invalid rate if the AP emits zero length SSID ie. As this
+		 * scan information is useless, ignore it
+		 */
+		if (!be16_to_cpu(scan_info->channel) || !scan_info->rate[0]) {
+			pr_debug("%s: invalid scan info\n", __func__);
+			continue;
+		}
+
 		found = 0;
 		oldest = NULL;
 		list_for_each_entry(target, &wl->network_list, list) {
@@ -1687,10 +1698,6 @@ static void gelic_wl_scan_complete_event
 					 GFP_KERNEL);
 		if (!target->hwinfo) {
 			pr_info("%s: kzalloc failed\n", __func__);
-			i++;
-			scan_info_size += be16_to_cpu(scan_info->size);
-			scan_info = (void *)scan_info +
-				be16_to_cpu(scan_info->size);
 			continue;
 		}
 		/* copy hw scan info */
@@ -1709,10 +1716,6 @@ static void gelic_wl_scan_complete_event
 			if (scan_info->ext_rate[r])
 				target->rate_ext_len++;
 		list_move_tail(&target->list, &wl->network_list);
-		/* bump pointer */
-		i++;
-		scan_info_size += be16_to_cpu(scan_info->size);
-		scan_info = (void *)scan_info + be16_to_cpu(scan_info->size);
 	}
 	memset(&data, 0, sizeof(data));
 	wireless_send_event(port_to_netdev(wl_port(wl)), SIOCGIWSCAN, &data,

-- 
Masakazu MOKUNO

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 572 bytes --]


On Tue, 2008-03-11 at 13:15 +0900, Masakazu Mokuno wrote:
> Some implementations of the hidden SSID APs emit beacons which have the zero
> length SSID information element instead of SSID padded by null (\0) characters.
> If the firmware of the PS3 wireless hardware meets these beacons, it abandons parsing
> IEs. Thus guest OSes get the invalid scan information for the AP.
> 
> To work around this, ignore these scan informations from the list.

I'm not sure this is a good idea. Is there any way to get at decent
information from those beacons?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Dan Williams]

On Tue, 2008-03-11 at 17:18 +0100, Johannes Berg wrote:
> On Tue, 2008-03-11 at 13:15 +0900, Masakazu Mokuno wrote:
> > Some implementations of the hidden SSID APs emit beacons which have the zero
> > length SSID information element instead of SSID padded by null (\0) characters.
> > If the firmware of the PS3 wireless hardware meets these beacons, it abandons parsing
> > IEs. Thus guest OSes get the invalid scan information for the AP.
> > 
> > To work around this, ignore these scan informations from the list.
> 
> I'm not sure this is a good idea. Is there any way to get at decent
> information from those beacons?

Yeah, there are quite a few of these out there.  Ideally the firmware
wouldn't puke on it...

Dan

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[John W. Linville]

On Tue, Mar 11, 2008 at 12:50:37PM -0400, Dan Williams wrote:
> On Tue, 2008-03-11 at 17:18 +0100, Johannes Berg wrote:
> > On Tue, 2008-03-11 at 13:15 +0900, Masakazu Mokuno wrote:
> > > Some implementations of the hidden SSID APs emit beacons which have the zero
> > > length SSID information element instead of SSID padded by null (\0) characters.
> > > If the firmware of the PS3 wireless hardware meets these beacons, it abandons parsing
> > > IEs. Thus guest OSes get the invalid scan information for the AP.
> > > 
> > > To work around this, ignore these scan informations from the list.
> > 
> > I'm not sure this is a good idea. Is there any way to get at decent
> > information from those beacons?
> 
> Yeah, there are quite a few of these out there.  Ideally the firmware
> wouldn't puke on it...

Is a firmware update a possibility?  We've had these discussions
before.  I doubt Sony is any more excited about a firmware spin than
Intel has been.

Does this problem apply to probe responses as well?  (Do probe
responses from hidden SSID APs still exclude the SSID?  I would
guess not.)

As long as you can still associate w/ hidden SSID APs, I don't see
a big problem.  Even if you can't, no information for those APs is
probably better than bad information, no?

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Masakazu Mokuno]

On Tue, 11 Mar 2008 13:10:24 -0400
"John W. Linville" <linville@tuxdriver.com> wrote:

> On Tue, Mar 11, 2008 at 12:50:37PM -0400, Dan Williams wrote:
> > On Tue, 2008-03-11 at 17:18 +0100, Johannes Berg wrote:
> > > On Tue, 2008-03-11 at 13:15 +0900, Masakazu Mokuno wrote:
> > > > Some implementations of the hidden SSID APs emit beacons which have the zero
> > > > length SSID information element instead of SSID padded by null (\0) characters.
> > > > If the firmware of the PS3 wireless hardware meets these beacons, it abandons parsing
> > > > IEs. Thus guest OSes get the invalid scan information for the AP.
> > > > 
> > > > To work around this, ignore these scan informations from the list.
> > > 
> > > I'm not sure this is a good idea. Is there any way to get at decent
> > > information from those beacons?
> > 
> > Yeah, there are quite a few of these out there.  Ideally the firmware
> > wouldn't puke on it...
> 
> Is a firmware update a possibility?  We've had these discussions
> before.  I doubt Sony is any more excited about a firmware spin than
> Intel has been.

The fix is already scheduled and is under testing. The future update of
the system software for PS3 should have the fix.

> Does this problem apply to probe responses as well?  (Do probe
> responses from hidden SSID APs still exclude the SSID?  I would
> guess not.)

There has been a problem in parsing IEs, so if the received probe
responses include zero length IEs, they also suffer this bug.

> As long as you can still associate w/ hidden SSID APs, I don't see
> a big problem.  Even if you can't, no information for those APs is
> probably better than bad information, no?

I was anxious that the userland apps or the users may get confused.  Any
other thing harmful is not found.
OK, I withdraw this patch.

Thanks for the reviews.

-- 
Masakazu MOKUNO

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Masakazu Mokuno]

On Tue, 11 Mar 2008 17:18:07 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:

> 
> On Tue, 2008-03-11 at 13:15 +0900, Masakazu Mokuno wrote:
> > Some implementations of the hidden SSID APs emit beacons which have the zero
> > length SSID information element instead of SSID padded by null (\0) characters.
> > If the firmware of the PS3 wireless hardware meets these beacons, it abandons parsing
> > IEs. Thus guest OSes get the invalid scan information for the AP.
> > 
> > To work around this, ignore these scan informations from the list.
> 
> I'm not sure this is a good idea. Is there any way to get at decent
> information from those beacons?

Currently no.

-- 
Masakazu MOKUNO

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]


> > Does this problem apply to probe responses as well?  (Do probe
> > responses from hidden SSID APs still exclude the SSID?  I would
> > guess not.)
> 
> There has been a problem in parsing IEs, so if the received probe
> responses include zero length IEs, they also suffer this bug.
> 
> > As long as you can still associate w/ hidden SSID APs, I don't see
> > a big problem.  Even if you can't, no information for those APs is
> > probably better than bad information, no?
> 
> I was anxious that the userland apps or the users may get confused.  Any
> other thing harmful is not found.
> OK, I withdraw this patch.

I think John actually meant that it would be better to have the patch
in. How does a "bad" scan result look like?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Masakazu Mokuno]

On Wed, 12 Mar 2008 09:25:10 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:

> 
> > > Does this problem apply to probe responses as well?  (Do probe
> > > responses from hidden SSID APs still exclude the SSID?  I would
> > > guess not.)
> > 
> > There has been a problem in parsing IEs, so if the received probe
> > responses include zero length IEs, they also suffer this bug.
> > 
> > > As long as you can still associate w/ hidden SSID APs, I don't see
> > > a big problem.  Even if you can't, no information for those APs is
> > > probably better than bad information, no?
> > 
> > I was anxious that the userland apps or the users may get confused.  Any
> > other thing harmful is not found.
> > OK, I withdraw this patch.
> 
> I think John actually meant that it would be better to have the patch
> in. How does a "bad" scan result look like?

Usually the zero length IE, the SSID element, is the first one of IE
list, all information in the successor elements is lost. What we can see
is bssid.

The current firmware gives the following result.  The Corega (00:0A:79)
AP was the one which sent zero length SSID if it was in hidden SSID mode.

> iwlist wlan0 scanning
> wlan0     Scan completed :
>           Cell 01 - Address: 00:06:25:C6:B9:A7
>                     ESSID:"planexuser"
>                     Channel=1
>                     Bit Rates:1 Mb/s; 2 Mb/s; 5.5 Mb/s; 6 Mb/s; 9 Mb/s
>                               11 Mb/s; 12 Mb/s; 18 Mb/s; 24 Mb/s; 36 Mb/s
>                               48 Mb/s; 54 Mb/s
>                     Encryption key:on
>                     Mode:Master
>                     Signal level=100/100  
>                     IE: IEEE 802.11i/WPA2 Version 1
>                         Group Cipher : CCMP
>                         Pairwise Ciphers (1) : CCMP
>                         Authentication Suites (1) : PSK
>           Cell 02 - Address: 00:0A:79:A5:D2:E2
>                     ESSID:""
>                     Channel:0
>                     Encryption key:on
>                     Mode:Master
>                     Signal level=100/100  


With fixed firmware:

> wlan0     Scan completed :
>           Cell 01 - Address: 00:06:25:C6:B9:A7
>                     ESSID:"planexuser"
>                     Channel=1
>                     Bit Rates:1 Mb/s; 2 Mb/s; 5.5 Mb/s; 6 Mb/s; 9 Mb/s
>                               11 Mb/s; 12 Mb/s; 18 Mb/s; 24 Mb/s; 36 Mb/s
>                               48 Mb/s; 54 Mb/s
>                     Encryption key:on
>                     Mode:Master
>                     Signal level=100/100  
>                     IE: IEEE 802.11i/WPA2 Version 1
>                         Group Cipher : CCMP
>                         Pairwise Ciphers (1) : CCMP
>                         Authentication Suites (1) : PSK
>           Cell 02 - Address: 00:0A:79:A5:D2:E2
>                     ESSID:""
>                     Channel:1
>                     Bit Rates:1 Mb/s; 2 Mb/s; 5.5 Mb/s; 11 Mb/s; 6 Mb/s
>                               9 Mb/s; 12 Mb/s; 18 Mb/s; 24 Mb/s; 36 Mb/s
>                               48 Mb/s; 54 Mb/s
>                     Encryption key:on
>                     Mode:Master
>                     Signal level=100/100  
>                     IE: IEEE 802.11i/WPA2 Version 1
>                         Group Cipher : CCMP
>                         Pairwise Ciphers (1) : CCMP
>                         Authentication Suites (1) : PSK
> 


-- 
Masakazu MOKUNO

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Dan Williams]

On Wed, 2008-03-12 at 19:26 +0900, Masakazu Mokuno wrote:
> On Wed, 12 Mar 2008 09:25:10 +0100
> Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> > 
> > > > Does this problem apply to probe responses as well?  (Do probe
> > > > responses from hidden SSID APs still exclude the SSID?  I would
> > > > guess not.)
> > > 
> > > There has been a problem in parsing IEs, so if the received probe
> > > responses include zero length IEs, they also suffer this bug.
> > > 
> > > > As long as you can still associate w/ hidden SSID APs, I don't see
> > > > a big problem.  Even if you can't, no information for those APs is
> > > > probably better than bad information, no?
> > > 
> > > I was anxious that the userland apps or the users may get confused.  Any
> > > other thing harmful is not found.
> > > OK, I withdraw this patch.
> > 
> > I think John actually meant that it would be better to have the patch
> > in. How does a "bad" scan result look like?
> 
> Usually the zero length IE, the SSID element, is the first one of IE
> list, all information in the successor elements is lost. What we can see
> is bssid.
> 
> The current firmware gives the following result.  The Corega (00:0A:79)
> AP was the one which sent zero length SSID if it was in hidden SSID mode.
> 
> > iwlist wlan0 scanning
> > wlan0     Scan completed :
> >           Cell 01 - Address: 00:06:25:C6:B9:A7
> >                     ESSID:"planexuser"
> >                     Channel=1
> >                     Bit Rates:1 Mb/s; 2 Mb/s; 5.5 Mb/s; 6 Mb/s; 9 Mb/s
> >                               11 Mb/s; 12 Mb/s; 18 Mb/s; 24 Mb/s; 36 Mb/s
> >                               48 Mb/s; 54 Mb/s
> >                     Encryption key:on
> >                     Mode:Master
> >                     Signal level=100/100  
> >                     IE: IEEE 802.11i/WPA2 Version 1
> >                         Group Cipher : CCMP
> >                         Pairwise Ciphers (1) : CCMP
> >                         Authentication Suites (1) : PSK
> >           Cell 02 - Address: 00:0A:79:A5:D2:E2
> >                     ESSID:""
> >                     Channel:0
> >                     Encryption key:on
> >                     Mode:Master
> >                     Signal level=100/100  

Ewww.  You're 100% correct; we certainly shouldn't be sending a scan
result like this to userspace.  Therefore:

Acked-by: Dan Williams <dcbw@redhat.com>

> 
> With fixed firmware:
> 
> > wlan0     Scan completed :
> >           Cell 01 - Address: 00:06:25:C6:B9:A7
> >                     ESSID:"planexuser"
> >                     Channel=1
> >                     Bit Rates:1 Mb/s; 2 Mb/s; 5.5 Mb/s; 6 Mb/s; 9 Mb/s
> >                               11 Mb/s; 12 Mb/s; 18 Mb/s; 24 Mb/s; 36 Mb/s
> >                               48 Mb/s; 54 Mb/s
> >                     Encryption key:on
> >                     Mode:Master
> >                     Signal level=100/100  
> >                     IE: IEEE 802.11i/WPA2 Version 1
> >                         Group Cipher : CCMP
> >                         Pairwise Ciphers (1) : CCMP
> >                         Authentication Suites (1) : PSK
> >           Cell 02 - Address: 00:0A:79:A5:D2:E2
> >                     ESSID:""
> >                     Channel:1
> >                     Bit Rates:1 Mb/s; 2 Mb/s; 5.5 Mb/s; 11 Mb/s; 6 Mb/s
> >                               9 Mb/s; 12 Mb/s; 18 Mb/s; 24 Mb/s; 36 Mb/s
> >                               48 Mb/s; 54 Mb/s
> >                     Encryption key:on
> >                     Mode:Master
> >                     Signal level=100/100  
> >                     IE: IEEE 802.11i/WPA2 Version 1
> >                         Group Cipher : CCMP
> >                         Pairwise Ciphers (1) : CCMP
> >                         Authentication Suites (1) : PSK
> > 
> 
> 

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]


> > >           Cell 02 - Address: 00:0A:79:A5:D2:E2
> > >                     ESSID:""
> > >                     Channel:0
> > >                     Encryption key:on
> > >                     Mode:Master
> > >                     Signal level=100/100  
> 
> Ewww.  You're 100% correct; we certainly shouldn't be sending a scan
> result like this to userspace.  Therefore:

I agree. Can you, however, tell whether the firmware has been fixed and
only skip these for broken firmware?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]


On Wed, 2008-03-12 at 15:31 +0100, Johannes Berg wrote:
> > > >           Cell 02 - Address: 00:0A:79:A5:D2:E2
> > > >                     ESSID:""
> > > >                     Channel:0
> > > >                     Encryption key:on
> > > >                     Mode:Master
> > > >                     Signal level=100/100  
> > 
> > Ewww.  You're 100% correct; we certainly shouldn't be sending a scan
> > result like this to userspace.  Therefore:
> 
> I agree. Can you, however, tell whether the firmware has been fixed and
> only skip these for broken firmware?

Oh, never mind, the patch will already do that. I agree with that
completely :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] PS3: gelic: ignore scan info from zero SSID beacons

[Masakazu Mokuno]

On Wed, 12 Mar 2008 15:43:11 +0100
Johannes Berg <johannes@sipsolutions.net> wrote:

> 
> On Wed, 2008-03-12 at 15:31 +0100, Johannes Berg wrote:
> > > > >           Cell 02 - Address: 00:0A:79:A5:D2:E2
> > > > >                     ESSID:""
> > > > >                     Channel:0
> > > > >                     Encryption key:on
> > > > >                     Mode:Master
> > > > >                     Signal level=100/100  
> > > 
> > > Ewww.  You're 100% correct; we certainly shouldn't be sending a scan
> > > result like this to userspace.  Therefore:
> > 
> > I agree. Can you, however, tell whether the firmware has been fixed and
> > only skip these for broken firmware?
> 
> Oh, never mind, the patch will already do that. I agree with that
> completely :)

Dan, Johannes, thanks for your reviewing!

So now I again submit this patch although I said once that I withdrew.

Please apply this patch for 2.6.25.

Thanks in advance.

-- 
Masakazu MOKUNO