RE: [patch] mwifiex: potential integer underflow in mwifiex_ret_wmm_get_status()

[Bing Zhao <bzhao@xxxxxxxxxxx>]

Hi Dan,

Thanks for your comments.

> > 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.

You are right. The negative value counts as a large positive number while comparing against sizeof().
I can add a "const int hdr_size" variable to store the value of sizeof(...) and compare resp_len to hdr_size. The "sizeof(...)" has been used multiple times in this function, so I think it's worth adding a variable for it.


diff --git a/drivers/net/wireless/mwifiex/wmm.c b/drivers/net/wireless/mwifiex/wmm.c
index 95fa359..c97df5a 100644
--- a/drivers/net/wireless/mwifiex/wmm.c
+++ b/drivers/net/wireless/mwifiex/wmm.c
@@ -707,10 +707,10 @@ int mwifiex_ret_wmm_get_status(struct mwifiex_private *priv,
                               const struct host_cmd_ds_command *resp)
 {
        u8 *curr = (u8 *) &resp->params.get_wmm_status;
-       uint16_t resp_len = le16_to_cpu(resp->size), tlv_len;
+       int resp_len = le16_to_cpu(resp->size), tlv_len;
        int valid = true;
-
        struct mwifiex_ie_types_data *tlv_hdr;
+       const int hdr_size = sizeof(tlv_hdr->header);
        struct mwifiex_ie_types_wmm_queue_status *tlv_wmm_qstatus;
        struct ieee_types_wmm_parameter *wmm_param_ie = NULL;
        struct mwifiex_wmm_ac_status *ac_status;
@@ -718,7 +718,7 @@ int mwifiex_ret_wmm_get_status(struct mwifiex_private *priv,
        dev_dbg(priv->adapter->dev, "info: WMM: WMM_GET_STATUS cmdresp received: %d\n",
                resp_len);

-       while ((resp_len >= sizeof(tlv_hdr->header)) && valid) {
+       while (resp_len >= hdr_size && valid) {
                tlv_hdr = (struct mwifiex_ie_types_data *) curr;
                tlv_len = le16_to_cpu(tlv_hdr->header.len);

@@ -772,8 +772,8 @@ int mwifiex_ret_wmm_get_status(struct mwifiex_private *priv,
                        break;
                }

-               curr += (tlv_len + sizeof(tlv_hdr->header));
-               resp_len -= (tlv_len + sizeof(tlv_hdr->header));
+               curr += tlv_len + hdr_size;
+               resp_len -= tlv_len + hdr_size;
        }

        mwifiex_wmm_setup_queue_priorities(priv, wmm_param_ie);


Thanks,
Bing
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html