2020-06-30 04:38:53

by Sean Wang

[permalink] [raw]
Subject: [PATCH 1/3] mt76: mt7663u: fix memory leak in set key

From: Sean Wang <[email protected]>

Fix memory leak in set key.

Fixes: eb99cc95c3b6 ("mt76: mt7615: introduce mt7663u support")
Signed-off-by: Sean Wang <[email protected]>
---
.../net/wireless/mediatek/mt76/mt7615/usb.c | 22 +++++++++++++------
1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/usb.c b/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
index 0ba28d37c414..96a081be108e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
@@ -165,12 +165,16 @@ __mt7663u_mac_set_key(struct mt7615_dev *dev,

lockdep_assert_held(&dev->mt76.mutex);

- if (!sta)
- return -EINVAL;
+ if (!sta) {
+ err = -EINVAL;
+ goto out;
+ }

cipher = mt7615_mac_get_cipher(key->cipher);
- if (cipher == MT_CIPHER_NONE)
- return -EOPNOTSUPP;
+ if (cipher == MT_CIPHER_NONE) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }

wcid = &wd->sta->wcid;

@@ -178,19 +182,23 @@ __mt7663u_mac_set_key(struct mt7615_dev *dev,
err = mt7615_mac_wtbl_update_key(dev, wcid, key->key, key->keylen,
cipher, key->cmd);
if (err < 0)
- return err;
+ goto out;

err = mt7615_mac_wtbl_update_pk(dev, wcid, cipher, key->keyidx,
key->cmd);
if (err < 0)
- return err;
+ goto out;

if (key->cmd == SET_KEY)
wcid->cipher |= BIT(cipher);
else
wcid->cipher &= ~BIT(cipher);

- return 0;
+out:
+ kfree(key->key);
+ kfree(wd);
+
+ return err;
}

void mt7663u_wtbl_work(struct work_struct *work)
--
2.25.1


2020-06-30 04:41:30

by Sean Wang

[permalink] [raw]
Subject: [PATCH 3/3] mt76: mt7615: fix potential memory leak in mcu message handler

From: Sean Wang <[email protected]>

Fix potential memory leak in mcu message handler on error condition.

Fixes: 0e6a29e477f3 ("mt76: mt7615: add support to read temperature from mcu")
Signed-off-by: Sean Wang <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index 1b46cccd93c5..58e3838a3dba 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -183,8 +183,10 @@ mt7615_mcu_parse_response(struct mt7615_dev *dev, int cmd,
struct mt7615_mcu_rxd *rxd = (struct mt7615_mcu_rxd *)skb->data;
int ret = 0;

- if (seq != rxd->seq)
- return -EAGAIN;
+ if (seq != rxd->seq) {
+ ret = -EAGAIN;
+ goto out;
+ }

switch (cmd) {
case MCU_CMD_PATCH_SEM_CONTROL:
@@ -215,6 +217,7 @@ mt7615_mcu_parse_response(struct mt7615_dev *dev, int cmd,
default:
break;
}
+out:
dev_kfree_skb(skb);

return ret;
--
2.25.1

2020-06-30 07:08:11

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH 1/3] mt76: mt7663u: fix memory leak in set key

> From: Sean Wang <[email protected]>
>
> Fix memory leak in set key.
>

Acked-by: Lorenzo Bianconi <[email protected]>

> Fixes: eb99cc95c3b6 ("mt76: mt7615: introduce mt7663u support")
> Signed-off-by: Sean Wang <[email protected]>
> ---
> .../net/wireless/mediatek/mt76/mt7615/usb.c | 22 +++++++++++++------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/usb.c b/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
> index 0ba28d37c414..96a081be108e 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
> @@ -165,12 +165,16 @@ __mt7663u_mac_set_key(struct mt7615_dev *dev,
>
> lockdep_assert_held(&dev->mt76.mutex);
>
> - if (!sta)
> - return -EINVAL;
> + if (!sta) {
> + err = -EINVAL;
> + goto out;
> + }
>
> cipher = mt7615_mac_get_cipher(key->cipher);
> - if (cipher == MT_CIPHER_NONE)
> - return -EOPNOTSUPP;
> + if (cipher == MT_CIPHER_NONE) {
> + err = -EOPNOTSUPP;
> + goto out;
> + }
>
> wcid = &wd->sta->wcid;
>
> @@ -178,19 +182,23 @@ __mt7663u_mac_set_key(struct mt7615_dev *dev,
> err = mt7615_mac_wtbl_update_key(dev, wcid, key->key, key->keylen,
> cipher, key->cmd);
> if (err < 0)
> - return err;
> + goto out;
>
> err = mt7615_mac_wtbl_update_pk(dev, wcid, cipher, key->keyidx,
> key->cmd);
> if (err < 0)
> - return err;
> + goto out;
>
> if (key->cmd == SET_KEY)
> wcid->cipher |= BIT(cipher);
> else
> wcid->cipher &= ~BIT(cipher);
>
> - return 0;
> +out:
> + kfree(key->key);
> + kfree(wd);
> +
> + return err;
> }
>
> void mt7663u_wtbl_work(struct work_struct *work)
> --
> 2.25.1


Attachments:
(No filename) (1.74 kB)
signature.asc (235.00 B)
Download all attachments

2020-06-30 07:18:45

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH 3/3] mt76: mt7615: fix potential memory leak in mcu message handler

> From: Sean Wang <[email protected]>
>
> Fix potential memory leak in mcu message handler on error condition.
>

Acked-by: Lorenzo Bianconi <[email protected]>

> Fixes: 0e6a29e477f3 ("mt76: mt7615: add support to read temperature from mcu")
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> index 1b46cccd93c5..58e3838a3dba 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> @@ -183,8 +183,10 @@ mt7615_mcu_parse_response(struct mt7615_dev *dev, int cmd,
> struct mt7615_mcu_rxd *rxd = (struct mt7615_mcu_rxd *)skb->data;
> int ret = 0;
>
> - if (seq != rxd->seq)
> - return -EAGAIN;
> + if (seq != rxd->seq) {
> + ret = -EAGAIN;
> + goto out;
> + }
>
> switch (cmd) {
> case MCU_CMD_PATCH_SEM_CONTROL:
> @@ -215,6 +217,7 @@ mt7615_mcu_parse_response(struct mt7615_dev *dev, int cmd,
> default:
> break;
> }
> +out:
> dev_kfree_skb(skb);
>
> return ret;
> --
> 2.25.1


Attachments:
(No filename) (1.23 kB)
signature.asc (235.00 B)
Download all attachments

2020-06-30 07:26:08

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH 1/3] mt76: mt7663u: fix memory leak in set key

> From: Sean Wang <[email protected]>
>
> Fix memory leak in set key.
>
> Fixes: eb99cc95c3b6 ("mt76: mt7615: introduce mt7663u support")
> Signed-off-by: Sean Wang <[email protected]>
> ---
> .../net/wireless/mediatek/mt76/mt7615/usb.c | 22 +++++++++++++------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/usb.c b/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
> index 0ba28d37c414..96a081be108e 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/usb.c
> @@ -165,12 +165,16 @@ __mt7663u_mac_set_key(struct mt7615_dev *dev,
>
> lockdep_assert_held(&dev->mt76.mutex);
>
> - if (!sta)
> - return -EINVAL;
> + if (!sta) {
> + err = -EINVAL;
> + goto out;
> + }
>
> cipher = mt7615_mac_get_cipher(key->cipher);
> - if (cipher == MT_CIPHER_NONE)
> - return -EOPNOTSUPP;
> + if (cipher == MT_CIPHER_NONE) {
> + err = -EOPNOTSUPP;
> + goto out;
> + }
>
> wcid = &wd->sta->wcid;
>
> @@ -178,19 +182,23 @@ __mt7663u_mac_set_key(struct mt7615_dev *dev,
> err = mt7615_mac_wtbl_update_key(dev, wcid, key->key, key->keylen,
> cipher, key->cmd);
> if (err < 0)
> - return err;
> + goto out;
>
> err = mt7615_mac_wtbl_update_pk(dev, wcid, cipher, key->keyidx,
> key->cmd);
> if (err < 0)
> - return err;
> + goto out;
>
> if (key->cmd == SET_KEY)
> wcid->cipher |= BIT(cipher);
> else
> wcid->cipher &= ~BIT(cipher);
>
> - return 0;
> +out:
> + kfree(key->key);
> + kfree(wd);

Actually we do not need to free wd since it is done in mt7663u_wtbl_work()

> +
> + return err;
> }
>
> void mt7663u_wtbl_work(struct work_struct *work)
> --
> 2.25.1


Attachments:
(No filename) (1.77 kB)
signature.asc (235.00 B)
Download all attachments

2020-06-30 22:10:15

by Sean Wang

[permalink] [raw]
Subject: Re: [PATCH 1/3] mt76: mt7663u: fix memory leak in set key

From: Sean Wang <[email protected]>

Thanks, I'll have the next version to remove the unneeded kfree.

>
>> +
>> + return err;
>> }
>>
>> void mt7663u_wtbl_work(struct work_struct *work)