2011-05-16 11:10:03

by Roland Vossen

[permalink] [raw]
Subject: Re: [PATCH] staging: brcm80211: fix cast to pointer from integer

Greg, please also merge this to the staging-linus branch since it might
resolve an instability issue.

On 05/15/2011 10:45 PM, Clemens Noss wrote:
> bcm_pktq_flush and related functions only ever get 0 or a pointer for
> arg, so make it a pointer.
>
> This might fix a crash on 64bit.

Acked-by: Roland Vossen <[email protected]>

>
> Signed-off-by: Clemens Noss<[email protected]>
> ---
> drivers/staging/brcm80211/brcmfmac/dhd_sdio.c | 2 +-
> drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c | 4 ++--
> drivers/staging/brcm80211/brcmsmac/wlc_main.c | 4 ++--
> drivers/staging/brcm80211/include/bcmutils.h | 6 +++---
> drivers/staging/brcm80211/util/bcmutils.c | 4 ++--
> 5 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c b/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
> index 8683eeb..a71c6f8 100644
> --- a/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
> +++ b/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
> @@ -2845,7 +2845,7 @@ void dhd_bus_stop(struct dhd_bus *bus, bool enforce_mutex)
> dhdsdio_clkctl(bus, CLK_SDONLY, false);
>
> /* Clear the data packet queues */
> - bcm_pktq_flush(&bus->txq, true, NULL, 0);
> + bcm_pktq_flush(&bus->txq, true, NULL, NULL);
>
> /* Clear any held glomming stuff */
> if (bus->glomd)
> diff --git a/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c b/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
> index 5b041a6..0d93bd6 100644
> --- a/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
> +++ b/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
> @@ -1225,7 +1225,7 @@ void wlc_ampdu_shm_upd(struct ampdu_info *ampdu)
> /*
> * callback function that helps flushing ampdu packets from a priority queue
> */
> -static bool cb_del_ampdu_pkt(void *p, int arg_a)
> +static bool cb_del_ampdu_pkt(void *p, void *arg_a)
> {
> struct sk_buff *mpdu = (struct sk_buff *)p;
> struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(mpdu);
> @@ -1269,7 +1269,7 @@ void wlc_ampdu_flush(struct wlc_info *wlc,
> ampdu_pars.tid = tid;
> for (prec = 0; prec< pq->num_prec; prec++) {
> bcm_pktq_pflush(pq, prec, true, cb_del_ampdu_pkt,
> - (int)&ampdu_pars);
> + (void *)&ampdu_pars);
> }
> wlc_inval_dma_pkts(wlc->hw, sta, dma_cb_fn_ampdu);
> }
> diff --git a/drivers/staging/brcm80211/brcmsmac/wlc_main.c b/drivers/staging/brcm80211/brcmsmac/wlc_main.c
> index 2fed0eb..926b01a 100644
> --- a/drivers/staging/brcm80211/brcmsmac/wlc_main.c
> +++ b/drivers/staging/brcm80211/brcmsmac/wlc_main.c
> @@ -2520,7 +2520,7 @@ uint wlc_down(struct wlc_info *wlc)
>
> /* flush tx queues */
> for (qi = wlc->tx_queues; qi != NULL; qi = qi->next) {
> - bcm_pktq_flush(&qi->q, true, NULL, 0);
> + bcm_pktq_flush(&qi->q, true, NULL, NULL);
> }
>
> callbacks += wlc_bmac_down_finish(wlc->hw);
> @@ -8135,7 +8135,7 @@ void wlc_wait_for_tx_completion(struct wlc_info *wlc, bool drop)
> {
> /* flush packet queue when requested */
> if (drop)
> - bcm_pktq_flush(&wlc->pkt_queue->q, false, NULL, 0);
> + bcm_pktq_flush(&wlc->pkt_queue->q, false, NULL, NULL);
>
> /* wait for queue and DMA fifos to run dry */
> while (!pktq_empty(&wlc->pkt_queue->q) ||
> diff --git a/drivers/staging/brcm80211/include/bcmutils.h b/drivers/staging/brcm80211/include/bcmutils.h
> index aa00e83..1d57934 100644
> --- a/drivers/staging/brcm80211/include/bcmutils.h
> +++ b/drivers/staging/brcm80211/include/bcmutils.h
> @@ -74,7 +74,7 @@
> #define PKTQ_PREC_ITER(pq, prec) for (prec = (pq)->num_prec - 1; prec>= 0; prec--)
>
> /* fn(pkt, arg). return true if pkt belongs to if */
> - typedef bool(*ifpkt_cb_t) (void *, int);
> + typedef bool(*ifpkt_cb_t) (void *, void *);
>
> /* operations on a specific precedence in packet queue */
>
> @@ -100,7 +100,7 @@ extern void bcm_pkt_buf_free_skb(struct sk_buff *skb);
>
> /* Empty the queue at particular precedence level */
> extern void bcm_pktq_pflush(struct pktq *pq, int prec,
> - bool dir, ifpkt_cb_t fn, int arg);
> + bool dir, ifpkt_cb_t fn, void *arg);
>
> /* operations on a set of precedences in packet queue */
>
> @@ -127,7 +127,7 @@ extern void bcm_pktq_init(struct pktq *pq, int num_prec, int max_len);
> /* prec_out may be NULL if caller is not interested in return value */
> extern struct sk_buff *bcm_pktq_peek_tail(struct pktq *pq, int *prec_out);
> extern void bcm_pktq_flush(struct pktq *pq, bool dir,
> - ifpkt_cb_t fn, int arg);
> + ifpkt_cb_t fn, void *arg);
>
> /* externs */
> /* packet */
> diff --git a/drivers/staging/brcm80211/util/bcmutils.c b/drivers/staging/brcm80211/util/bcmutils.c
> index e185e28..43e5bb3 100644
> --- a/drivers/staging/brcm80211/util/bcmutils.c
> +++ b/drivers/staging/brcm80211/util/bcmutils.c
> @@ -231,7 +231,7 @@ EXPORT_SYMBOL(bcm_pktq_pdeq_tail);
>
> void
> bcm_pktq_pflush(struct pktq *pq, int prec, bool dir,
> - ifpkt_cb_t fn, int arg)
> + ifpkt_cb_t fn, void *arg)
> {
> struct pktq_prec *q;
> struct sk_buff *p, *prev = NULL;
> @@ -263,7 +263,7 @@ bcm_pktq_pflush(struct pktq *pq, int prec, bool dir,
> EXPORT_SYMBOL(bcm_pktq_pflush);
>
> void bcm_pktq_flush(struct pktq *pq, bool dir,
> - ifpkt_cb_t fn, int arg)
> + ifpkt_cb_t fn, void *arg)
> {
> int prec;
> for (prec = 0; prec< pq->num_prec; prec++)




2011-05-17 10:53:51

by Roland Vossen

[permalink] [raw]
Subject: Re: [PATCH] staging: brcm80211: fix cast to pointer from integer

>> Greg, please also merge this to the staging-linus branch since it
>> might resolve an instability issue.
>
> It "might"?

Correction: I tested it now. Indeed it resolves a problem (segmentation
fault) upon dissociation on amd64.

> Is this a regression? Do people see the problem on the .38 kernel as
> well?

People on the .38 kernel see the problem even worse: they experience a
crash on dissociation on both i386 and amd64.

> It's way too late for anything but severe regressions to be sent to
> Linus at this point in time...

Understood. Since behavior has not regressed, I think this fix does not
qualify to be sent to Linus. Perhaps we can get it into a
compat-wireless package.

Thanks, Roland.


2011-05-16 20:34:57

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] staging: brcm80211: fix cast to pointer from integer

On Mon, May 16, 2011 at 01:09:50PM +0200, Roland Vossen wrote:
> Greg, please also merge this to the staging-linus branch since it
> might resolve an instability issue.

It "might"?

Is this a regression? Do people see the problem on the .38 kernel as
well?

It's way too late for anything but severe regressions to be sent to
Linus at this point in time...

thanks,

greg k-h