Skip to content

Commit

Permalink
USB: cxacru: check device isn't being removed during sysfs calls
Browse files Browse the repository at this point in the history
It is possible for usb_get_intfdata() to return NULL if
sysfs is accessed while the module is being unloaded or
the device is being removed.

Move the access code to an inline function in usbatm.h,
and return -ENODEV if any of the pointers are NULL.

It should not be possible for the instance data or atm
device to be invalid until after unbind() completes and
the sysfs attributes have been removed.

Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Simon Arlott authored and Greg Kroah-Hartman committed Mar 2, 2010
1 parent 5d0a9c7 commit 9fc950d
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 20 deletions.
48 changes: 28 additions & 20 deletions drivers/usb/atm/cxacru.c
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,12 @@ static DEVICE_ATTR(_name, S_IWUSR | S_IRUGO, \
static ssize_t cxacru_sysfs_show_##_name(struct device *dev, \
struct device_attribute *attr, char *buf) \
{ \
struct usb_interface *intf = to_usb_interface(dev); \
struct usbatm_data *usbatm_instance = usb_get_intfdata(intf); \
struct cxacru_data *instance = usbatm_instance->driver_data; \
struct cxacru_data *instance = to_usbatm_driver_data(\
to_usb_interface(dev)); \
\
if (instance == NULL) \
return -ENODEV; \
\
return cxacru_sysfs_showattr_##_type(instance->card_info[_value], buf); \
} \
CXACRU__ATTR_INIT(_name)
Expand Down Expand Up @@ -288,22 +291,28 @@ static ssize_t cxacru_sysfs_showattr_MODU(u32 value, char *buf)
static ssize_t cxacru_sysfs_show_mac_address(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct usb_interface *intf = to_usb_interface(dev);
struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
struct atm_dev *atm_dev = usbatm_instance->atm_dev;
struct cxacru_data *instance = to_usbatm_driver_data(
to_usb_interface(dev));

return snprintf(buf, PAGE_SIZE, "%pM\n", atm_dev->esi);
if (instance == NULL || instance->usbatm->atm_dev == NULL)
return -ENODEV;

return snprintf(buf, PAGE_SIZE, "%pM\n",
instance->usbatm->atm_dev->esi);
}

static ssize_t cxacru_sysfs_show_adsl_state(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct usb_interface *intf = to_usb_interface(dev);
struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
struct cxacru_data *instance = usbatm_instance->driver_data;
u32 value = instance->card_info[CXINF_LINE_STARTABLE];

static char *str[] = { "running", "stopped" };
struct cxacru_data *instance = to_usbatm_driver_data(
to_usb_interface(dev));
u32 value;

if (instance == NULL)
return -ENODEV;

value = instance->card_info[CXINF_LINE_STARTABLE];
if (unlikely(value >= ARRAY_SIZE(str)))
return snprintf(buf, PAGE_SIZE, "%u\n", value);
return snprintf(buf, PAGE_SIZE, "%s\n", str[value]);
Expand All @@ -312,9 +321,8 @@ static ssize_t cxacru_sysfs_show_adsl_state(struct device *dev,
static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
struct usb_interface *intf = to_usb_interface(dev);
struct usbatm_data *usbatm_instance = usb_get_intfdata(intf);
struct cxacru_data *instance = usbatm_instance->driver_data;
struct cxacru_data *instance = to_usbatm_driver_data(
to_usb_interface(dev));
int ret;
int poll = -1;
char str_cmd[8];
Expand All @@ -328,13 +336,16 @@ static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev,
return -EINVAL;
ret = 0;

if (instance == NULL)
return -ENODEV;

if (mutex_lock_interruptible(&instance->adsl_state_serialize))
return -ERESTARTSYS;

if (!strcmp(str_cmd, "stop") || !strcmp(str_cmd, "restart")) {
ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_STOP, NULL, 0, NULL, 0);
if (ret < 0) {
atm_err(usbatm_instance, "change adsl state:"
atm_err(instance->usbatm, "change adsl state:"
" CHIP_ADSL_LINE_STOP returned %d\n", ret);

ret = -EIO;
Expand All @@ -354,7 +365,7 @@ static ssize_t cxacru_sysfs_store_adsl_state(struct device *dev,
if (!strcmp(str_cmd, "start") || !strcmp(str_cmd, "restart")) {
ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
if (ret < 0) {
atm_err(usbatm_instance, "change adsl state:"
atm_err(instance->usbatm, "change adsl state:"
" CHIP_ADSL_LINE_START returned %d\n", ret);

ret = -EIO;
Expand Down Expand Up @@ -649,9 +660,6 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
{
struct cxacru_data *instance = usbatm_instance->driver_data;
struct usb_interface *intf = usbatm_instance->usb_intf;
/*
struct atm_dev *atm_dev = usbatm_instance->atm_dev;
*/
int ret;
int start_polling = 1;

Expand Down
1 change: 1 addition & 0 deletions drivers/usb/atm/usbatm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1333,6 +1333,7 @@ void usbatm_usb_disconnect(struct usb_interface *intf)
if (instance->atm_dev) {
sysfs_remove_link(&instance->atm_dev->class_dev.kobj, "device");
atm_dev_deregister(instance->atm_dev);
instance->atm_dev = NULL;
}

usbatm_put_instance(instance); /* taken in usbatm_usb_probe */
Expand Down
15 changes: 15 additions & 0 deletions drivers/usb/atm/usbatm.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,4 +204,19 @@ struct usbatm_data {
struct urb *urbs[0];
};

static inline void *to_usbatm_driver_data(struct usb_interface *intf)
{
struct usbatm_data *usbatm_instance;

if (intf == NULL)
return NULL;

usbatm_instance = usb_get_intfdata(intf);

if (usbatm_instance == NULL) /* set NULL before unbind() */
return NULL;

return usbatm_instance->driver_data; /* set NULL after unbind() */
}

#endif /* _USBATM_H_ */

0 comments on commit 9fc950d

Please sign in to comment.