From 3f7086978fc0193eff24a77d8b57ac4debc088fa Mon Sep 17 00:00:00 2001 From: Larry Finger Date: Tue, 4 Sep 2007 14:14:20 -0500 Subject: [PATCH] bcm43xx: Fix cancellation of work queue crashes A crash upon booting that is caused by bcm43xx has been reported [1] and found to be due to a work queue being reinitialized while work on that queue is still pending. This fix modifies the shutdown of work queues and prevents periodic work from being requeued during shutdown. With this patch, no more crashes on reboot were observed by the original reporter. I do not get that particular failure on my system; however, when running a large number of ifdown/ifup sequences, my system would kernel panic with the 'caps lock' light blinking at roughly a 1 Hz rate. In addition, there were infrequent failures in the firmware that resulted in 'IRQ READY TIMEOUT' errors. With this patch, no more of the first type of failure occur, and incidence of the second type is greatly reduced. [1] http://bugzilla.kernel.org/show_bug.cgi?id=8937 Signed-off-by: Larry Finger Acked-by: Michael Buesch Signed-off-by: John W. Linville --- drivers/net/wireless/bcm43xx/bcm43xx_main.c | 28 ++++++++++++++++++++-------- drivers/net/wireless/bcm43xx/bcm43xx_main.h | 2 +- drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c | 2 +- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c b/drivers/net/wireless/bcm43xx/bcm43xx_main.c index c5d6753a55e..dfbd01eaaf3 100644 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c @@ -3183,6 +3183,9 @@ static void bcm43xx_periodic_work_handler(struct work_struct *work) unsigned long orig_trans_start = 0; mutex_lock(&bcm->mutex); + /* keep from doing and rearming periodic work if shutting down */ + if (bcm43xx_status(bcm) == BCM43xx_STAT_UNINIT) + goto unlock_mutex; if (unlikely(bcm->periodic_state % 60 == 0)) { /* Periodic work will take a long time, so we want it to * be preemtible. @@ -3228,14 +3231,10 @@ static void bcm43xx_periodic_work_handler(struct work_struct *work) mmiowb(); bcm->periodic_state++; spin_unlock_irqrestore(&bcm->irq_lock, flags); +unlock_mutex: mutex_unlock(&bcm->mutex); } -void bcm43xx_periodic_tasks_delete(struct bcm43xx_private *bcm) -{ - cancel_rearming_delayed_work(&bcm->periodic_work); -} - void bcm43xx_periodic_tasks_setup(struct bcm43xx_private *bcm) { struct delayed_work *work = &bcm->periodic_work; @@ -3285,6 +3284,14 @@ static int bcm43xx_rng_init(struct bcm43xx_private *bcm) return err; } +void bcm43xx_cancel_work(struct bcm43xx_private *bcm) +{ + /* The system must be unlocked when this routine is entered. + * If not, the next 2 steps may deadlock */ + cancel_work_sync(&bcm->restart_work); + cancel_delayed_work_sync(&bcm->periodic_work); +} + static int bcm43xx_shutdown_all_wireless_cores(struct bcm43xx_private *bcm) { int ret = 0; @@ -3321,7 +3328,12 @@ static void bcm43xx_free_board(struct bcm43xx_private *bcm) { bcm43xx_rng_exit(bcm); bcm43xx_sysfs_unregister(bcm); - bcm43xx_periodic_tasks_delete(bcm); + + mutex_lock(&(bcm)->mutex); + bcm43xx_set_status(bcm, BCM43xx_STAT_UNINIT); + mutex_unlock(&(bcm)->mutex); + + bcm43xx_cancel_work(bcm); mutex_lock(&(bcm)->mutex); bcm43xx_shutdown_all_wireless_cores(bcm); @@ -4016,7 +4028,7 @@ static int bcm43xx_net_stop(struct net_device *net_dev) err = bcm43xx_disable_interrupts_sync(bcm); assert(!err); bcm43xx_free_board(bcm); - flush_scheduled_work(); + bcm43xx_cancel_work(bcm); return 0; } @@ -4148,9 +4160,9 @@ static void bcm43xx_chip_reset(struct work_struct *work) struct bcm43xx_phyinfo *phy; int err = -ENODEV; + bcm43xx_cancel_work(bcm); mutex_lock(&(bcm)->mutex); if (bcm43xx_status(bcm) == BCM43xx_STAT_INITIALIZED) { - bcm43xx_periodic_tasks_delete(bcm); phy = bcm43xx_current_phy(bcm); err = bcm43xx_select_wireless_core(bcm, phy->type); if (!err) diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.h b/drivers/net/wireless/bcm43xx/bcm43xx_main.h index c8f3c532bab..14cfbeb582e 100644 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.h +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.h @@ -122,7 +122,7 @@ void bcm43xx_wireless_core_reset(struct bcm43xx_private *bcm, int connect_phy); void bcm43xx_mac_suspend(struct bcm43xx_private *bcm); void bcm43xx_mac_enable(struct bcm43xx_private *bcm); -void bcm43xx_periodic_tasks_delete(struct bcm43xx_private *bcm); +void bcm43xx_cancel_work(struct bcm43xx_private *bcm); void bcm43xx_periodic_tasks_setup(struct bcm43xx_private *bcm); void bcm43xx_controller_restart(struct bcm43xx_private *bcm, const char *reason); diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c b/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c index c71b998a369..8ab5f93d192 100644 --- a/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c +++ b/drivers/net/wireless/bcm43xx/bcm43xx_sysfs.c @@ -327,7 +327,7 @@ static ssize_t bcm43xx_attr_phymode_store(struct device *dev, goto out; } - bcm43xx_periodic_tasks_delete(bcm); + bcm43xx_cancel_work(bcm); mutex_lock(&(bcm)->mutex); err = bcm43xx_select_wireless_core(bcm, phytype); if (!err) -- cgit v1.2.3 From 53c5725581cce8a29925afd4eae71fa8c7ce551f Mon Sep 17 00:00:00 2001 From: Masakazu Mokuno Date: Fri, 14 Sep 2007 14:35:38 -0400 Subject: As struct iw_point is bi-directional payload, we should copy back the content on return from ioctl calls Signed-off-by: Masakazu Mokuno Signed-off-by: John W. Linville --- fs/compat_ioctl.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index a6c9078af12..5a5b7116cef 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -2311,8 +2311,10 @@ static int do_wireless_ioctl(unsigned int fd, unsigned int cmd, unsigned long ar struct iwreq __user *iwr_u; struct iw_point __user *iwp; struct compat_iw_point __user *iwp_u; - compat_caddr_t pointer; + compat_caddr_t pointer_u; + void __user *pointer; __u16 length, flags; + int ret; iwr_u = compat_ptr(arg); iwp_u = (struct compat_iw_point __user *) &iwr_u->u.data; @@ -2330,17 +2332,29 @@ static int do_wireless_ioctl(unsigned int fd, unsigned int cmd, unsigned long ar sizeof(iwr->ifr_ifrn.ifrn_name))) return -EFAULT; - if (__get_user(pointer, &iwp_u->pointer) || + if (__get_user(pointer_u, &iwp_u->pointer) || __get_user(length, &iwp_u->length) || __get_user(flags, &iwp_u->flags)) return -EFAULT; - if (__put_user(compat_ptr(pointer), &iwp->pointer) || + if (__put_user(compat_ptr(pointer_u), &iwp->pointer) || __put_user(length, &iwp->length) || __put_user(flags, &iwp->flags)) return -EFAULT; - return sys_ioctl(fd, cmd, (unsigned long) iwr); + ret = sys_ioctl(fd, cmd, (unsigned long) iwr); + + if (__get_user(pointer, &iwp->pointer) || + __get_user(length, &iwp->length) || + __get_user(flags, &iwp->flags)) + return -EFAULT; + + if (__put_user(ptr_to_compat(pointer), &iwp_u->pointer) || + __put_user(length, &iwp_u->length) || + __put_user(flags, &iwp_u->flags)) + return -EFAULT; + + return ret; } /* Since old style bridge ioctl's endup using SIOCDEVPRIVATE -- cgit v1.2.3