| Message ID | 20130925085729.GC6661@elgon.mountain (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Dan, > If "resp_len" gets set to negative then it counts as a high positive value. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > I spotted this reviewing the int => bool changes, but I don't have the > hardware and can't test it. Thanks for spotting this potential integer underflow problem. I think we can change the 'resp_len' variable type to a signed integer to fix this issue. Thanks, Bing > > diff --git a/drivers/net/wireless/mwifiex/wmm.c > b/drivers/net/wireless/mwifiex/wmm.c > index 2e8f9cd..3c6ee3a 100644 > --- a/drivers/net/wireless/mwifiex/wmm.c > +++ b/drivers/net/wireless/mwifiex/wmm.c > @@ -772,6 +772,8 @@ int mwifiex_ret_wmm_get_status(struct > mwifiex_private *priv, > break; > } > > + if (resp_len < tlv_len + sizeof(tlv_hdr->header)) > + break; > curr += (tlv_len + sizeof(tlv_hdr->header)); > resp_len -= (tlv_len + sizeof(tlv_hdr->header)); > } -- 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
On Wed, Sep 25, 2013 at 09:25:46AM -0700, Bing Zhao wrote: > Hi Dan, > > > If "resp_len" gets set to negative then it counts as a high positive value. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > I spotted this reviewing the int => bool changes, but I don't have the > > hardware and can't test it. > > Thanks for spotting this potential integer underflow problem. > > I think we can change the 'resp_len' variable type to a signed integer > to fix this issue. No, that doesn't work because the comparison against sizeof() get's promoted to size_t. In other words, negative values still count as large positive values. regards, dan carpenter -- 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
diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c index 2e8f9cd..3c6ee3a 100644 --- a/drivers/net/wireless/mwifiex/wmm.c +++ b/drivers/net/wireless/mwifiex/wmm.c @@ -772,6 +772,8 @@ int mwifiex_ret_wmm_get_status(struct mwifiex_private *priv, break; } + if (resp_len < tlv_len + sizeof(tlv_hdr->header)) + break; curr += (tlv_len + sizeof(tlv_hdr->header)); resp_len -= (tlv_len + sizeof(tlv_hdr->header)); }
If "resp_len" gets set to negative then it counts as a high positive value. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- I spotted this reviewing the int => bool changes, but I don't have the hardware and can't test it. -- 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