| Message ID | 1405137435-14045-1-git-send-email-bzhao@marvell.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Fri, Jul 11, 2014 at 8:57 PM, Bing Zhao <bzhao@marvell.com> wrote: > It's already been taken cared by the 'default' case in the > switch/case below. > > Reported-by: Paul Stewart <pstew@chromium.org> > Signed-off-by: Bing Zhao <bzhao@marvell.com> > Signed-off-by: Avinash Patil <patila@marvell.com> > --- > drivers/net/wireless/mwifiex/tdls.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/wireless/mwifiex/tdls.c b/drivers/net/wireless/mwifiex/tdls.c > index a414161..57f972d 100644 > --- a/drivers/net/wireless/mwifiex/tdls.c > +++ b/drivers/net/wireless/mwifiex/tdls.c > @@ -791,19 +791,15 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv, > return; > > peer = buf + ETH_ALEN; > - action = *(buf + sizeof(struct ethhdr) + 2); > > - /* just handle TDLS setup request/response/confirm */ > - if (action > WLAN_TDLS_SETUP_CONFIRM) I'm going to assume that the intent of the original test was to avoid allocation of sta_ptr below in the case where the action was invalid. Is it okay to allocate this pointer and not do anything with it below? > + sta_ptr = mwifiex_add_sta_entry(priv, peer); > + if (!sta_ptr) > return; > > + action = *(buf + sizeof(struct ethhdr) + 2); > dev_dbg(priv->adapter->dev, > "rx:tdls action: peer=%pM, action=%d\n", peer, action); > > - sta_ptr = mwifiex_add_sta_entry(priv, peer); > - if (!sta_ptr) > - return; > - > switch (action) { > case WLAN_TDLS_SETUP_REQUEST: > if (len < (sizeof(struct ethhdr) + TDLS_REQ_FIX_LEN)) > -- > 1.8.2.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Paul, Thanks for your comment. > > @@ -791,19 +791,15 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv, > > return; > > > > peer = buf + ETH_ALEN; > > - action = *(buf + sizeof(struct ethhdr) + 2); > > > > - /* just handle TDLS setup request/response/confirm */ > > - if (action > WLAN_TDLS_SETUP_CONFIRM) > I'm going to assume that the intent of the original test was to avoid > allocation of sta_ptr below in the case where the action was invalid. > Is it okay to allocate this pointer and not do anything with it below? I've sent v2 patch. Could you please review? Thanks, Bing > > + sta_ptr = mwifiex_add_sta_entry(priv, peer); > > + if (!sta_ptr) > > return; > > > > + action = *(buf + sizeof(struct ethhdr) + 2); > > dev_dbg(priv->adapter->dev, > > "rx:tdls action: peer=%pM, action=%d\n", peer, action); > > > > - sta_ptr = mwifiex_add_sta_entry(priv, peer); > > - if (!sta_ptr) > > - return; > > - > > switch (action) { > > case WLAN_TDLS_SETUP_REQUEST: > > if (len < (sizeof(struct ethhdr) + TDLS_REQ_FIX_LEN)) > > -- > > 1.8.2.3 > >
diff --git a/drivers/net/wireless/mwifiex/tdls.c b/drivers/net/wireless/mwifiex/tdls.c index a414161..57f972d 100644 --- a/drivers/net/wireless/mwifiex/tdls.c +++ b/drivers/net/wireless/mwifiex/tdls.c @@ -791,19 +791,15 @@ void mwifiex_process_tdls_action_frame(struct mwifiex_private *priv, return; peer = buf + ETH_ALEN; - action = *(buf + sizeof(struct ethhdr) + 2); - /* just handle TDLS setup request/response/confirm */ - if (action > WLAN_TDLS_SETUP_CONFIRM) + sta_ptr = mwifiex_add_sta_entry(priv, peer); + if (!sta_ptr) return; + action = *(buf + sizeof(struct ethhdr) + 2); dev_dbg(priv->adapter->dev, "rx:tdls action: peer=%pM, action=%d\n", peer, action); - sta_ptr = mwifiex_add_sta_entry(priv, peer); - if (!sta_ptr) - return; - switch (action) { case WLAN_TDLS_SETUP_REQUEST: if (len < (sizeof(struct ethhdr) + TDLS_REQ_FIX_LEN))