All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rtlwifi: rtl8192cu: Fix NULL dereference BUG when using new_id
@ 2013-02-06  0:00 Larry Finger
  2013-02-06  0:18 ` Ben Hutchings
  0 siblings, 1 reply; 6+ messages in thread
From: Larry Finger @ 2013-02-06  0:00 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry Finger, netdev, Stable

When the new_id entry is used for a foreign USB device, rtlwifi BUGS with
a NULL pointer dereference.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>
---

John,

Although this patch should be backported to stable kernels, the new_id
feature is rarely used, thus the patch should not have any particular
priority.

Larry
---

 drivers/net/wireless/rtlwifi/usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/rtlwifi/usb.c b/drivers/net/wireless/rtlwifi/usb.c
index d42bbe2..77a7517 100644
--- a/drivers/net/wireless/rtlwifi/usb.c
+++ b/drivers/net/wireless/rtlwifi/usb.c
@@ -977,6 +977,9 @@ int rtl_usb_probe(struct usb_interface *intf,
 	rtl_dbgp_flag_init(hw);
 	/* Init IO handler */
 	_rtl_usb_io_handler_init(&udev->dev, hw);
+	if (!rtlpriv->cfg || !rtlpriv->cfg->ops ||
+	    !rtlpriv->cfg->ops->read_chip_version)
+		return -ENODEV;
 	rtlpriv->cfg->ops->read_chip_version(hw);
 	/*like read eeprom and so on */
 	rtlpriv->cfg->ops->read_eeprom_info(hw);
-- 
1.8.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtlwifi: rtl8192cu: Fix NULL dereference BUG when using new_id
  2013-02-06  0:00 [PATCH] rtlwifi: rtl8192cu: Fix NULL dereference BUG when using new_id Larry Finger
@ 2013-02-06  0:18 ` Ben Hutchings
  2013-02-06  0:44   ` Larry Finger
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2013-02-06  0:18 UTC (permalink / raw)
  To: Larry Finger; +Cc: linville, linux-wireless, netdev, Stable

On Tue, 2013-02-05 at 18:00 -0600, Larry Finger wrote:
> When the new_id entry is used for a foreign USB device, rtlwifi BUGS with
> a NULL pointer dereference.
[...]

So set no_dynamic_id in the usb_driver structures.

(But I wonder why USB behaves differently from PCI, which requires that
the dynamic ID's driver_data value (defaulting to 0) matches a value
used in a static ID entry.)

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtlwifi: rtl8192cu: Fix NULL dereference BUG when using new_id
  2013-02-06  0:18 ` Ben Hutchings
@ 2013-02-06  0:44   ` Larry Finger
  2013-02-06  1:16     ` Ben Hutchings
  0 siblings, 1 reply; 6+ messages in thread
From: Larry Finger @ 2013-02-06  0:44 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: linville, linux-wireless, netdev, Stable

On 02/05/2013 06:18 PM, Ben Hutchings wrote:
> On Tue, 2013-02-05 at 18:00 -0600, Larry Finger wrote:
>> When the new_id entry is used for a foreign USB device, rtlwifi BUGS with
>> a NULL pointer dereference.
> [...]
>
> So set no_dynamic_id in the usb_driver structures.
>
> (But I wonder why USB behaves differently from PCI, which requires that
> the dynamic ID's driver_data value (defaulting to 0) matches a value
> used in a static ID entry.)

I don't know why USB differs from PCI, but we do need the dynamic ID here as 
there are always new IDs being issued. One of the criteria for adding the ID to 
the table is that it works OK with dynamic addition. These devices are 
frequently reported by users that do not have the skills to build their own kernel.

Larry



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtlwifi: rtl8192cu: Fix NULL dereference BUG when using new_id
  2013-02-06  0:44   ` Larry Finger
@ 2013-02-06  1:16     ` Ben Hutchings
  2013-02-06  9:01         ` Johannes Berg
  0 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2013-02-06  1:16 UTC (permalink / raw)
  To: Larry Finger; +Cc: linville, linux-wireless, netdev, Stable

On Tue, 2013-02-05 at 18:44 -0600, Larry Finger wrote:
> On 02/05/2013 06:18 PM, Ben Hutchings wrote:
> > On Tue, 2013-02-05 at 18:00 -0600, Larry Finger wrote:
> >> When the new_id entry is used for a foreign USB device, rtlwifi BUGS with
> >> a NULL pointer dereference.
> > [...]
> >
> > So set no_dynamic_id in the usb_driver structures.
> >
> > (But I wonder why USB behaves differently from PCI, which requires that
> > the dynamic ID's driver_data value (defaulting to 0) matches a value
> > used in a static ID entry.)
> 
> I don't know why USB differs from PCI, but we do need the dynamic ID here as 
> there are always new IDs being issued. One of the criteria for adding the ID to 
> the table is that it works OK with dynamic addition. These devices are 
> frequently reported by users that do not have the skills to build their own kernel.

But since there is no way to set the driver_info for a new USB ID
(again, unlike PCI), your change will reject all dynamic IDs.  (And in
any case, if the USB core were changed to allow setting driver_info,
userland would have difficulty providing a valid pointer!)

It looks like the driver_info is really driver-specific data used to
share a probe function between multiple drivers.  But you could add
per-driver probe functions that pass the correct rtl_hal_cfg as an extra
argument to rtl_usb_probe(), and then dynamic IDs should work.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtlwifi: rtl8192cu: Fix NULL dereference BUG when using new_id
@ 2013-02-06  9:01         ` Johannes Berg
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2013-02-06  9:01 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Larry Finger, linville, linux-wireless, netdev, Stable

On Wed, 2013-02-06 at 01:16 +0000, Ben Hutchings wrote:

> > I don't know why USB differs from PCI, but we do need the dynamic ID here as 
> > there are always new IDs being issued. One of the criteria for adding the ID to 
> > the table is that it works OK with dynamic addition. These devices are 
> > frequently reported by users that do not have the skills to build their own kernel.
> 
> But since there is no way to set the driver_info for a new USB ID
> (again, unlike PCI), your change will reject all dynamic IDs.  (And in
> any case, if the USB core were changed to allow setting driver_info,
> userland would have difficulty providing a valid pointer!)
> 
> It looks like the driver_info is really driver-specific data used to
> share a probe function between multiple drivers.  But you could add
> per-driver probe functions that pass the correct rtl_hal_cfg as an extra
> argument to rtl_usb_probe(), and then dynamic IDs should work.

Some (PCI?) drivers had/have numbers instead of pointers, so userland
can provide a number and it then looks up the pointer in an array.
That's only slightly indirected but makes new_id more useful in that
case.

johannes


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] rtlwifi: rtl8192cu: Fix NULL dereference BUG when using new_id
@ 2013-02-06  9:01         ` Johannes Berg
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2013-02-06  9:01 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Larry Finger, linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Stable

On Wed, 2013-02-06 at 01:16 +0000, Ben Hutchings wrote:

> > I don't know why USB differs from PCI, but we do need the dynamic ID here as 
> > there are always new IDs being issued. One of the criteria for adding the ID to 
> > the table is that it works OK with dynamic addition. These devices are 
> > frequently reported by users that do not have the skills to build their own kernel.
> 
> But since there is no way to set the driver_info for a new USB ID
> (again, unlike PCI), your change will reject all dynamic IDs.  (And in
> any case, if the USB core were changed to allow setting driver_info,
> userland would have difficulty providing a valid pointer!)
> 
> It looks like the driver_info is really driver-specific data used to
> share a probe function between multiple drivers.  But you could add
> per-driver probe functions that pass the correct rtl_hal_cfg as an extra
> argument to rtl_usb_probe(), and then dynamic IDs should work.

Some (PCI?) drivers had/have numbers instead of pointers, so userland
can provide a number and it then looks up the pointer in an array.
That's only slightly indirected but makes new_id more useful in that
case.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-02-06  9:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-06  0:00 [PATCH] rtlwifi: rtl8192cu: Fix NULL dereference BUG when using new_id Larry Finger
2013-02-06  0:18 ` Ben Hutchings
2013-02-06  0:44   ` Larry Finger
2013-02-06  1:16     ` Ben Hutchings
2013-02-06  9:01       ` Johannes Berg
2013-02-06  9:01         ` Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.