[RFC AR6000 patch V4] Set hardware unavailable during wext_ioctl when the suspend or rfkill is send

Michael Trimarchi trimarchi at gandalf.sssup.it
Tue Mar 31 18:16:25 CEST 2009


Hi

Michael Trimarchi wrote:
> The ioctl wext etc, seems to be broken because they don't take any lock
> during shutdown. If the user do an echo to the state variabile of the
> rfkill during the network scanning the system can go in panic. It
> introduces a semaphore variable to the ar priv data.
>
> Signed-off-by: Michael Trimarchi <michael at panicking.kicks-ass.org>
> ---
>  drivers/ar6000/ar6000/ar6000_drv.c   |    5 +-
>  drivers/ar6000/ar6000/ar6000_drv.h   |    1 +
>  drivers/ar6000/ar6000/wireless_ext.c |  136 ++++++++++++++++++++++++----------
>  3 files changed, 101 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/ar6000/ar6000/ar6000_drv.c b/drivers/ar6000/ar6000/ar6000_drv.c
> index 21504f2..87194d4 100644
> --- a/drivers/ar6000/ar6000/ar6000_drv.c
> +++ b/drivers/ar6000/ar6000/ar6000_drv.c
> @@ -902,6 +902,7 @@ ar6000_avail_ev(HTC_HANDLE HTCHandle)
>      AR_DEBUG_PRINTF("ar6000_avail: name=%s htcTarget=0x%x, dev=0x%x (%d), ar=0x%x\n",
>                      dev->name, (A_UINT32)HTCHandle, (A_UINT32)dev, device_index,
>                      (A_UINT32)ar);
> +    init_rwsem(&ar->arHwAvail);
>  }
>
>  static void ar6000_target_failure(void *Instance, A_STATUS Status)
> @@ -942,7 +943,9 @@ static void
>  ar6000_unavail_ev(void *Instance)
>  {
>      AR_SOFTC_T *ar = (AR_SOFTC_T *)Instance;
> -        /* NULL out it's entry in the global list */
> +    /* NULL out it's entry in the global list */
> +
> +    down_write(&ar->arHwAvail);
>      ar6000_devices[ar->arDeviceIndex] = NULL;
>      ar6000_destroy(ar->arNetDev, 1);
>  }
> diff --git a/drivers/ar6000/ar6000/ar6000_drv.h b/drivers/ar6000/ar6000/ar6000_drv.h
> index 784f158..c8c273c 100644
> --- a/drivers/ar6000/ar6000/ar6000_drv.h
> +++ b/drivers/ar6000/ar6000/ar6000_drv.h
> @@ -205,6 +205,7 @@ typedef struct ar6_softc {
>      HTC_HANDLE              arHtcTarget;
>      void                    *arHifDevice;
>      spinlock_t              arLock;
> +    struct rw_semaphore     arHwAvail;
>      struct semaphore        arSem;
>      int                     arRxBuffers[WMI_PRI_MAX_COUNT];
>      int                     arSsidLen;
> diff --git a/drivers/ar6000/ar6000/wireless_ext.c b/drivers/ar6000/ar6000/wireless_ext.c
> index d9a5920..55f78d6 100644
> --- a/drivers/ar6000/ar6000/wireless_ext.c
> +++ b/drivers/ar6000/ar6000/wireless_ext.c
> @@ -25,6 +25,20 @@ extern unsigned int wmitimeout;
>  extern A_WAITQUEUE_HEAD arEvent;
>  extern wait_queue_head_t ar6000_scan_queue;
>
> +#define DECLARE_LOCKED_VERSION(_name) \
> +int lock_##_name(struct net_device *dev, struct iw_request_info *info, \
> +                void *p, char *data) \
> +{ \
> +	AR_SOFTC_T *ar = (AR_SOFTC_T *)netdev_priv(dev);	\
> +	int ret = -EAGAIN;					\
> +								\
> +	if (down_read_trylock(&ar->arHwAvail)) {		\
> +		ret = _name(dev, info, p, data);		\
> +		up_read(&ar->arHwAvail);			\
> +	}							\
> +	return ret;						\
> +}
> +
>  /*
>   * Encode a WPA or RSN information element as a custom
>   * element using the hostap format.
> @@ -1849,21 +1863,63 @@ ar6000_set_quality(struct iw_quality *iq, A_INT8 rssi)
>      iq->updated = 7;
>  }
>
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwname)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwfreq)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwmode)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwfreq)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwmode)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwsens)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwsens)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwrange)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwap)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwap)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_iwaplist)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwscan)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwscan)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwessid)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwessid)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwrate)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwrate)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwtxpow)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwtxpow)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwretry)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwretry)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwencode)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwencode)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwpower)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwpower)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwgenie)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwgenie)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwauth)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwauth)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_siwencodeext)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_giwencodeext)
> +
> +/* private handler */
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_setparam)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_getparam)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_setkey)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_setwmmparams)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_delkey)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_getwmmparams)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_setoptie)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_setmlme)
> +DECLARE_LOCKED_VERSION(ar6000_ioctl_addpmkid)
>
>  /* Structures to export the Wireless Handlers */
>  static const iw_handler ath_handlers[] = {
>      (iw_handler) NULL,                          /* SIOCSIWCOMMIT */
> -    (iw_handler) ar6000_ioctl_giwname,          /* SIOCGIWNAME */
> +    (iw_handler) lock_ar6000_ioctl_giwname,          /* SIOCGIWNAME */
>      (iw_handler) NULL,                          /* SIOCSIWNWID */
>      (iw_handler) NULL,                          /* SIOCGIWNWID */
> -    (iw_handler) ar6000_ioctl_siwfreq,          /* SIOCSIWFREQ */
> -    (iw_handler) ar6000_ioctl_giwfreq,          /* SIOCGIWFREQ */
> -    (iw_handler) ar6000_ioctl_siwmode,          /* SIOCSIWMODE */
> -    (iw_handler) ar6000_ioctl_giwmode,          /* SIOCGIWMODE */
> -    (iw_handler) ar6000_ioctl_siwsens,          /* SIOCSIWSENS */
> -    (iw_handler) ar6000_ioctl_giwsens,          /* SIOCGIWSENS */
> +    (iw_handler) lock_ar6000_ioctl_siwfreq,          /* SIOCSIWFREQ */
> +    (iw_handler) lock_ar6000_ioctl_giwfreq,          /* SIOCGIWFREQ */
> +    (iw_handler) lock_ar6000_ioctl_siwmode,          /* SIOCSIWMODE */
> +    (iw_handler) lock_ar6000_ioctl_giwmode,          /* SIOCGIWMODE */
> +    (iw_handler) lock_ar6000_ioctl_siwsens,          /* SIOCSIWSENS */
> +    (iw_handler) lock_ar6000_ioctl_giwsens,          /* SIOCGIWSENS */
>      (iw_handler) NULL /* not _used */,          /* SIOCSIWRANGE */
> -    (iw_handler) ar6000_ioctl_giwrange,         /* SIOCGIWRANGE */
> +    (iw_handler) lock_ar6000_ioctl_giwrange,         /* SIOCGIWRANGE */
>      (iw_handler) NULL /* not used */,           /* SIOCSIWPRIV */
>      (iw_handler) NULL /* kernel code */,        /* SIOCGIWPRIV */
>      (iw_handler) NULL /* not used */,           /* SIOCSIWSTATS */
> @@ -1872,53 +1928,53 @@ static const iw_handler ath_handlers[] = {
>      (iw_handler) NULL,                          /* SIOCGIWSPY */
>      (iw_handler) NULL,                          /* SIOCSIWTHRSPY */
>      (iw_handler) NULL,                          /* SIOCGIWTHRSPY */
> -    (iw_handler) ar6000_ioctl_siwap,            /* SIOCSIWAP */
> -    (iw_handler) ar6000_ioctl_giwap,            /* SIOCGIWAP */
> +    (iw_handler) lock_ar6000_ioctl_siwap,            /* SIOCSIWAP */
> +    (iw_handler) lock_ar6000_ioctl_giwap,            /* SIOCGIWAP */
>      (iw_handler) NULL,                          /* -- hole -- */
> -    (iw_handler) ar6000_ioctl_iwaplist,         /* SIOCGIWAPLIST */
> -    (iw_handler) ar6000_ioctl_siwscan,          /* SIOCSIWSCAN */
> -    (iw_handler) ar6000_ioctl_giwscan,          /* SIOCGIWSCAN */
> -    (iw_handler) ar6000_ioctl_siwessid,         /* SIOCSIWESSID */
> -    (iw_handler) ar6000_ioctl_giwessid,         /* SIOCGIWESSID */
> +    (iw_handler) lock_ar6000_ioctl_iwaplist,         /* SIOCGIWAPLIST */
> +    (iw_handler) lock_ar6000_ioctl_siwscan,          /* SIOCSIWSCAN */
> +    (iw_handler) lock_ar6000_ioctl_giwscan,          /* SIOCGIWSCAN */
> +    (iw_handler) lock_ar6000_ioctl_siwessid,         /* SIOCSIWESSID */
> +    (iw_handler) lock_ar6000_ioctl_giwessid,         /* SIOCGIWESSID */
>      (iw_handler) NULL,                          /* SIOCSIWNICKN */
>      (iw_handler) NULL,                          /* SIOCGIWNICKN */
>      (iw_handler) NULL,                          /* -- hole -- */
>      (iw_handler) NULL,                          /* -- hole -- */
> -    (iw_handler) ar6000_ioctl_siwrate,          /* SIOCSIWRATE */
> -    (iw_handler) ar6000_ioctl_giwrate,          /* SIOCGIWRATE */
> +    (iw_handler) lock_ar6000_ioctl_siwrate,          /* SIOCSIWRATE */
> +    (iw_handler) lock_ar6000_ioctl_giwrate,          /* SIOCGIWRATE */
>      (iw_handler) NULL,           /* SIOCSIWRTS */
>      (iw_handler) NULL,           /* SIOCGIWRTS */
>      (iw_handler) NULL,          /* SIOCSIWFRAG */
>      (iw_handler) NULL,          /* SIOCGIWFRAG */
> -    (iw_handler) ar6000_ioctl_siwtxpow,         /* SIOCSIWTXPOW */
> -    (iw_handler) ar6000_ioctl_giwtxpow,         /* SIOCGIWTXPOW */
> -    (iw_handler) ar6000_ioctl_siwretry,         /* SIOCSIWRETRY */
> -    (iw_handler) ar6000_ioctl_giwretry,         /* SIOCGIWRETRY */
> -    (iw_handler) ar6000_ioctl_siwencode,        /* SIOCSIWENCODE */
> -    (iw_handler) ar6000_ioctl_giwencode,        /* SIOCGIWENCODE */
> -    (iw_handler) ar6000_ioctl_siwpower,         /* SIOCSIWPOWER */
> -    (iw_handler) ar6000_ioctl_giwpower,         /* SIOCGIWPOWER */
> +    (iw_handler) lock_ar6000_ioctl_siwtxpow,         /* SIOCSIWTXPOW */
> +    (iw_handler) lock_ar6000_ioctl_giwtxpow,         /* SIOCGIWTXPOW */
> +    (iw_handler) lock_ar6000_ioctl_siwretry,         /* SIOCSIWRETRY */
> +    (iw_handler) lock_ar6000_ioctl_giwretry,         /* SIOCGIWRETRY */
> +    (iw_handler) lock_ar6000_ioctl_siwencode,        /* SIOCSIWENCODE */
> +    (iw_handler) lock_ar6000_ioctl_giwencode,        /* SIOCGIWENCODE */
> +    (iw_handler) lock_ar6000_ioctl_siwpower,         /* SIOCSIWPOWER */
> +    (iw_handler) lock_ar6000_ioctl_giwpower,         /* SIOCGIWPOWER */
>      (iw_handler) NULL,	/* -- hole -- */
>      (iw_handler) NULL,	/* -- hole -- */
> -    (iw_handler) ar6000_ioctl_siwgenie,	/* SIOCSIWGENIE */
> -    (iw_handler) ar6000_ioctl_giwgenie,	/* SIOCGIWGENIE */
> -    (iw_handler) ar6000_ioctl_siwauth,	/* SIOCSIWAUTH */
> -    (iw_handler) ar6000_ioctl_giwauth,	/* SIOCGIWAUTH */
> -    (iw_handler) ar6000_ioctl_siwencodeext,/* SIOCSIWENCODEEXT */
> -    (iw_handler) ar6000_ioctl_giwencodeext,/* SIOCGIWENCODEEXT */
> +    (iw_handler) lock_ar6000_ioctl_siwgenie,	/* SIOCSIWGENIE */
> +    (iw_handler) lock_ar6000_ioctl_giwgenie,	/* SIOCGIWGENIE */
> +    (iw_handler) lock_ar6000_ioctl_siwauth,	/* SIOCSIWAUTH */
> +    (iw_handler) lock_ar6000_ioctl_giwauth,	/* SIOCGIWAUTH */
> +    (iw_handler) lock_ar6000_ioctl_siwencodeext,/* SIOCSIWENCODEEXT */
> +    (iw_handler) lock_ar6000_ioctl_giwencodeext,/* SIOCGIWENCODEEXT */
>      (iw_handler) NULL,		/* SIOCSIWPMKSA */
>  };
>
>  static const iw_handler ath_priv_handlers[] = {
> -    (iw_handler) ar6000_ioctl_setparam,         /* SIOCWFIRSTPRIV+0 */
> -    (iw_handler) ar6000_ioctl_getparam,         /* SIOCWFIRSTPRIV+1 */
> -    (iw_handler) ar6000_ioctl_setkey,           /* SIOCWFIRSTPRIV+2 */
> -    (iw_handler) ar6000_ioctl_setwmmparams,     /* SIOCWFIRSTPRIV+3 */
> -    (iw_handler) ar6000_ioctl_delkey,           /* SIOCWFIRSTPRIV+4 */
> -    (iw_handler) ar6000_ioctl_getwmmparams,     /* SIOCWFIRSTPRIV+5 */
> -    (iw_handler) ar6000_ioctl_setoptie,         /* SIOCWFIRSTPRIV+6 */
> -    (iw_handler) ar6000_ioctl_setmlme,          /* SIOCWFIRSTPRIV+7 */
> -    (iw_handler) ar6000_ioctl_addpmkid,         /* SIOCWFIRSTPRIV+8 */
> +    (iw_handler) lock_ar6000_ioctl_setparam,         /* SIOCWFIRSTPRIV+0 */
> +    (iw_handler) lock_ar6000_ioctl_getparam,         /* SIOCWFIRSTPRIV+1 */
> +    (iw_handler) lock_ar6000_ioctl_setkey,           /* SIOCWFIRSTPRIV+2 */
> +    (iw_handler) lock_ar6000_ioctl_setwmmparams,     /* SIOCWFIRSTPRIV+3 */
> +    (iw_handler) lock_ar6000_ioctl_delkey,           /* SIOCWFIRSTPRIV+4 */
> +    (iw_handler) lock_ar6000_ioctl_getwmmparams,     /* SIOCWFIRSTPRIV+5 */
> +    (iw_handler) lock_ar6000_ioctl_setoptie,         /* SIOCWFIRSTPRIV+6 */
> +    (iw_handler) lock_ar6000_ioctl_setmlme,          /* SIOCWFIRSTPRIV+7 */
> +    (iw_handler) lock_ar6000_ioctl_addpmkid,         /* SIOCWFIRSTPRIV+8 */
>  };
>
>  #define IW_PRIV_TYPE_KEY \
> --
> 1.5.6.5
>
>
>
>   
I change again, maybe is not necessary but I was at the end.
I start to rewrite step-by-step and clean up some part of the driver 
preparing it for a upstream. Do you think that is it necessary?

Thanks for the support




More information about the openmoko-kernel mailing list