* [PATCH 0/3] smsc95xx enhancements
@ 2012-11-28 17:46 Steve Glendinning
2012-11-28 17:46 ` [PATCH 1/3] smsc95xx: fix suspend buffer overflow Steve Glendinning
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Steve Glendinning @ 2012-11-28 17:46 UTC (permalink / raw)
To: netdev; +Cc: Steve Glendinning
This patchset is a resubmission of 1 updated patch and two new
enhancements.
Please consider all for net-next
Steve Glendinning (3):
smsc95xx: fix suspend buffer overflow
smsc95xx: fix error handling in suspend failure case
smsc95xx: don't enable remote wakeup directly
drivers/net/usb/smsc95xx.c | 85 ++++++++++++++++++++------------------------
1 file changed, 39 insertions(+), 46 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] smsc95xx: fix suspend buffer overflow
2012-11-28 17:46 [PATCH 0/3] smsc95xx enhancements Steve Glendinning
@ 2012-11-28 17:46 ` Steve Glendinning
2012-11-28 17:46 ` [PATCH 2/3] smsc95xx: fix error handling in suspend failure case Steve Glendinning
2012-11-28 17:46 ` [PATCH 3/3] smsc95xx: don't enable remote wakeup directly Steve Glendinning
2 siblings, 0 replies; 5+ messages in thread
From: Steve Glendinning @ 2012-11-28 17:46 UTC (permalink / raw)
To: netdev; +Cc: Steve Glendinning, Bjorn Mork, Joe Perches
This patch fixes a buffer overflow introduced by bbd9f9e, where
the filter_mask array is accessed beyond its bounds.
Updated to also add a check for kzalloc failure, as reported by
Bjorn Mork and Joe Perches.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
Cc: Bjorn Mork <bjorn@mork.no>
Cc: Joe Perches <joe@perches.com>
---
drivers/net/usb/smsc95xx.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 79d495d..c397b3a 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1281,7 +1281,7 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
}
if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
- u32 *filter_mask = kzalloc(32, GFP_KERNEL);
+ u32 *filter_mask = kzalloc(sizeof(u32) * 32, GFP_KERNEL);
u32 command[2];
u32 offset[2];
u32 crc[4];
@@ -1290,6 +1290,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
LAN9500A_WUFF_NUM : LAN9500_WUFF_NUM;
int i, filter = 0;
+ if (!filter_mask) {
+ netdev_warn(dev->net, "Unable to allocate filter_mask\n");
+ return -ENOMEM;
+ }
+
memset(command, 0, sizeof(command));
memset(offset, 0, sizeof(offset));
memset(crc, 0, sizeof(crc));
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] smsc95xx: fix error handling in suspend failure case
2012-11-28 17:46 [PATCH 0/3] smsc95xx enhancements Steve Glendinning
2012-11-28 17:46 ` [PATCH 1/3] smsc95xx: fix suspend buffer overflow Steve Glendinning
@ 2012-11-28 17:46 ` Steve Glendinning
2012-11-29 2:04 ` David Miller
2012-11-28 17:46 ` [PATCH 3/3] smsc95xx: don't enable remote wakeup directly Steve Glendinning
2 siblings, 1 reply; 5+ messages in thread
From: Steve Glendinning @ 2012-11-28 17:46 UTC (permalink / raw)
To: netdev; +Cc: Steve Glendinning
This patch ensures that if we fail to suspend the LAN9500 device
we call usbnet_resume before returning failure, instead of
leaving the usbnet driver in an unusable state.
Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
drivers/net/usb/smsc95xx.c | 50 +++++++++++++++++++++++++-------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index c397b3a..ffeaf13 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1248,35 +1248,37 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
/* disable energy detect (link up) & wake up events */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- check_warn_return(ret, "Error reading WUCSR\n");
+ check_warn_goto_done(ret, "Error reading WUCSR\n");
val &= ~(WUCSR_MPEN_ | WUCSR_WAKE_EN_);
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- check_warn_return(ret, "Error writing WUCSR\n");
+ check_warn_goto_done(ret, "Error writing WUCSR\n");
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- check_warn_return(ret, "Error reading PM_CTRL\n");
+ check_warn_goto_done(ret, "Error reading PM_CTRL\n");
val &= ~(PM_CTL_ED_EN_ | PM_CTL_WOL_EN_);
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- check_warn_return(ret, "Error writing PM_CTRL\n");
+ check_warn_goto_done(ret, "Error writing PM_CTRL\n");
- return smsc95xx_enter_suspend2(dev);
+ ret = smsc95xx_enter_suspend2(dev);
+ goto done;
}
if (pdata->wolopts & WAKE_PHY) {
ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
(PHY_INT_MASK_ANEG_COMP_ | PHY_INT_MASK_LINK_DOWN_));
- check_warn_return(ret, "error enabling PHY wakeup ints\n");
+ check_warn_goto_done(ret, "error enabling PHY wakeup ints\n");
/* if link is down then configure EDPD and enter SUSPEND1,
* otherwise enter SUSPEND0 below
*/
if (!link_up) {
netdev_info(dev->net, "entering SUSPEND1 mode\n");
- return smsc95xx_enter_suspend1(dev);
+ ret = smsc95xx_enter_suspend1(dev);
+ goto done;
}
}
@@ -1292,7 +1294,8 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
if (!filter_mask) {
netdev_warn(dev->net, "Unable to allocate filter_mask\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto done;
}
memset(command, 0, sizeof(command));
@@ -1354,49 +1357,49 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
ret = smsc95xx_write_reg_nopm(dev, WUFF, filter_mask[i]);
if (ret < 0)
kfree(filter_mask);
- check_warn_return(ret, "Error writing WUFF\n");
+ check_warn_goto_done(ret, "Error writing WUFF\n");
}
kfree(filter_mask);
for (i = 0; i < (wuff_filter_count / 4); i++) {
ret = smsc95xx_write_reg_nopm(dev, WUFF, command[i]);
- check_warn_return(ret, "Error writing WUFF\n");
+ check_warn_goto_done(ret, "Error writing WUFF\n");
}
for (i = 0; i < (wuff_filter_count / 4); i++) {
ret = smsc95xx_write_reg_nopm(dev, WUFF, offset[i]);
- check_warn_return(ret, "Error writing WUFF\n");
+ check_warn_goto_done(ret, "Error writing WUFF\n");
}
for (i = 0; i < (wuff_filter_count / 2); i++) {
ret = smsc95xx_write_reg_nopm(dev, WUFF, crc[i]);
- check_warn_return(ret, "Error writing WUFF\n");
+ check_warn_goto_done(ret, "Error writing WUFF\n");
}
/* clear any pending pattern match packet status */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- check_warn_return(ret, "Error reading WUCSR\n");
+ check_warn_goto_done(ret, "Error reading WUCSR\n");
val |= WUCSR_WUFR_;
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- check_warn_return(ret, "Error writing WUCSR\n");
+ check_warn_goto_done(ret, "Error writing WUCSR\n");
}
if (pdata->wolopts & WAKE_MAGIC) {
/* clear any pending magic packet status */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- check_warn_return(ret, "Error reading WUCSR\n");
+ check_warn_goto_done(ret, "Error reading WUCSR\n");
val |= WUCSR_MPR_;
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- check_warn_return(ret, "Error writing WUCSR\n");
+ check_warn_goto_done(ret, "Error writing WUCSR\n");
}
/* enable/disable wakeup sources */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- check_warn_return(ret, "Error reading WUCSR\n");
+ check_warn_goto_done(ret, "Error reading WUCSR\n");
if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
netdev_info(dev->net, "enabling pattern match wakeup\n");
@@ -1415,11 +1418,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
}
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- check_warn_return(ret, "Error writing WUCSR\n");
+ check_warn_goto_done(ret, "Error writing WUCSR\n");
/* enable wol wakeup source */
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- check_warn_return(ret, "Error reading PM_CTRL\n");
+ check_warn_goto_done(ret, "Error reading PM_CTRL\n");
val |= PM_CTL_WOL_EN_;
@@ -1428,14 +1431,19 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
val |= PM_CTL_ED_EN_;
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- check_warn_return(ret, "Error writing PM_CTRL\n");
+ check_warn_goto_done(ret, "Error writing PM_CTRL\n");
/* enable receiver to enable frame reception */
smsc95xx_start_rx_path(dev, 1);
/* some wol options are enabled, so enter SUSPEND0 */
netdev_info(dev->net, "entering SUSPEND0 mode\n");
- return smsc95xx_enter_suspend0(dev);
+ ret = smsc95xx_enter_suspend0(dev);
+
+done:
+ if (ret)
+ usbnet_resume(intf);
+ return ret;
}
static int smsc95xx_resume(struct usb_interface *intf)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] smsc95xx: don't enable remote wakeup directly
2012-11-28 17:46 [PATCH 0/3] smsc95xx enhancements Steve Glendinning
2012-11-28 17:46 ` [PATCH 1/3] smsc95xx: fix suspend buffer overflow Steve Glendinning
2012-11-28 17:46 ` [PATCH 2/3] smsc95xx: fix error handling in suspend failure case Steve Glendinning
@ 2012-11-28 17:46 ` Steve Glendinning
2 siblings, 0 replies; 5+ messages in thread
From: Steve Glendinning @ 2012-11-28 17:46 UTC (permalink / raw)
To: netdev; +Cc: Steve Glendinning, Bjorn Mork
As pointed out by Bjorn Mork, the generic "usb" driver sets this
for us so no need to directly set it in this driver.
Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
Cc: Bjorn Mork <bjorn@mork.no>
---
drivers/net/usb/smsc95xx.c | 30 +++++-------------------------
1 file changed, 5 insertions(+), 25 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index ffeaf13..064df1a 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -154,25 +154,6 @@ static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
{
return __smsc95xx_write_reg(dev, index, data, 0);
}
-static int smsc95xx_set_feature(struct usbnet *dev, u32 feature)
-{
- if (WARN_ON_ONCE(!dev))
- return -EINVAL;
-
- return usbnet_write_cmd_nopm(dev, USB_REQ_SET_FEATURE,
- USB_RECIP_DEVICE, feature, 0,
- NULL, 0);
-}
-
-static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
-{
- if (WARN_ON_ONCE(!dev))
- return -EINVAL;
-
- return usbnet_write_cmd_nopm(dev, USB_REQ_CLEAR_FEATURE,
- USB_RECIP_DEVICE, feature,
- 0, NULL, 0);
-}
/* Loop until the read is completed with timeout
* called with phy_mutex held */
@@ -685,8 +666,13 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
{
struct usbnet *dev = netdev_priv(net);
struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ int ret;
pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
+
+ ret = device_set_wakeup_enable(&dev->udev->dev, pdata->wolopts);
+ check_warn_return(ret, "device_set_wakeup_enable error %d\n", ret);
+
return 0;
}
@@ -1160,8 +1146,6 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
check_warn_return(ret, "Error reading PM_CTRL\n");
- smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
-
return 0;
}
@@ -1204,8 +1188,6 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
check_warn_return(ret, "Error writing PM_CTRL\n");
- smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
-
return 0;
}
@@ -1456,8 +1438,6 @@ static int smsc95xx_resume(struct usb_interface *intf)
BUG_ON(!dev);
if (pdata->wolopts) {
- smsc95xx_clear_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
-
/* clear wake-up sources */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR\n");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] smsc95xx: fix error handling in suspend failure case
2012-11-28 17:46 ` [PATCH 2/3] smsc95xx: fix error handling in suspend failure case Steve Glendinning
@ 2012-11-29 2:04 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2012-11-29 2:04 UTC (permalink / raw)
To: steve.glendinning; +Cc: netdev
From: Steve Glendinning <steve.glendinning@shawell.net>
Date: Wed, 28 Nov 2012 17:46:58 +0000
> - check_warn_return(ret, "Error reading WUCSR\n");
> + check_warn_goto_done(ret, "Error reading WUCSR\n");
This is just another example of how error prone these macros
are, kill them.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-11-29 2:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-28 17:46 [PATCH 0/3] smsc95xx enhancements Steve Glendinning
2012-11-28 17:46 ` [PATCH 1/3] smsc95xx: fix suspend buffer overflow Steve Glendinning
2012-11-28 17:46 ` [PATCH 2/3] smsc95xx: fix error handling in suspend failure case Steve Glendinning
2012-11-29 2:04 ` David Miller
2012-11-28 17:46 ` [PATCH 3/3] smsc95xx: don't enable remote wakeup directly Steve Glendinning
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).