* [PATCH 1/4] net: caif: added cfserl_release function
2021-06-03 16:37 [PATCH 0/4] net: caif: fix 2 memory leaks Pavel Skripkin
@ 2021-06-03 16:38 ` Pavel Skripkin
2021-06-03 16:38 ` [PATCH 2/4] net: caif: add proper error handling Pavel Skripkin
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Pavel Skripkin @ 2021-06-03 16:38 UTC (permalink / raw)
To: davem, kuba, sjur.brandeland; +Cc: netdev, linux-kernel, Pavel Skripkin, stable
Added cfserl_release() function.
Cc: stable@vger.kernel.org
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
include/net/caif/cfserl.h | 1 +
net/caif/cfserl.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/include/net/caif/cfserl.h b/include/net/caif/cfserl.h
index 14a55e03bb3c..67cce8757175 100644
--- a/include/net/caif/cfserl.h
+++ b/include/net/caif/cfserl.h
@@ -9,4 +9,5 @@
#include <net/caif/caif_layer.h>
struct cflayer *cfserl_create(int instance, bool use_stx);
+void cfserl_release(struct cflayer *layer);
#endif
diff --git a/net/caif/cfserl.c b/net/caif/cfserl.c
index e11725a4bb0e..40cd57ad0a0f 100644
--- a/net/caif/cfserl.c
+++ b/net/caif/cfserl.c
@@ -31,6 +31,11 @@ static int cfserl_transmit(struct cflayer *layr, struct cfpkt *pkt);
static void cfserl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
int phyid);
+void cfserl_release(struct cflayer *layer)
+{
+ kfree(layer);
+}
+
struct cflayer *cfserl_create(int instance, bool use_stx)
{
struct cfserl *this = kzalloc(sizeof(struct cfserl), GFP_ATOMIC);
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] net: caif: add proper error handling
2021-06-03 16:37 [PATCH 0/4] net: caif: fix 2 memory leaks Pavel Skripkin
2021-06-03 16:38 ` [PATCH 1/4] net: caif: added cfserl_release function Pavel Skripkin
@ 2021-06-03 16:38 ` Pavel Skripkin
2021-06-03 16:39 ` [PATCH 3/4] net: caif: fix memory leak in caif_device_notify Pavel Skripkin
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Pavel Skripkin @ 2021-06-03 16:38 UTC (permalink / raw)
To: davem, kuba, sjur.brandeland; +Cc: netdev, linux-kernel, Pavel Skripkin, stable
caif_enroll_dev() can fail in some cases. Ingnoring
these cases can lead to memory leak due to not assigning
link_support pointer to anywhere.
Fixes: 7c18d2205ea7 ("caif: Restructure how link caif link layer enroll")
Cc: stable@vger.kernel.org
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
include/net/caif/caif_dev.h | 2 +-
include/net/caif/cfcnfg.h | 2 +-
net/caif/caif_dev.c | 8 +++++---
net/caif/cfcnfg.c | 16 +++++++++++-----
4 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/include/net/caif/caif_dev.h b/include/net/caif/caif_dev.h
index 48ecca8530ff..b655d8666f55 100644
--- a/include/net/caif/caif_dev.h
+++ b/include/net/caif/caif_dev.h
@@ -119,7 +119,7 @@ void caif_free_client(struct cflayer *adap_layer);
* The link_support layer is used to add any Link Layer specific
* framing.
*/
-void caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
+int caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
struct cflayer *link_support, int head_room,
struct cflayer **layer, int (**rcv_func)(
struct sk_buff *, struct net_device *,
diff --git a/include/net/caif/cfcnfg.h b/include/net/caif/cfcnfg.h
index 2aa5e91d8457..8819ff4db35a 100644
--- a/include/net/caif/cfcnfg.h
+++ b/include/net/caif/cfcnfg.h
@@ -62,7 +62,7 @@ void cfcnfg_remove(struct cfcnfg *cfg);
* @fcs: Specify if checksum is used in CAIF Framing Layer.
* @head_room: Head space needed by link specific protocol.
*/
-void
+int
cfcnfg_add_phy_layer(struct cfcnfg *cnfg,
struct net_device *dev, struct cflayer *phy_layer,
enum cfcnfg_phy_preference pref,
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index c10e5a55758d..fffbe41440b3 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -308,7 +308,7 @@ static void dev_flowctrl(struct net_device *dev, int on)
caifd_put(caifd);
}
-void caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
+int caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
struct cflayer *link_support, int head_room,
struct cflayer **layer,
int (**rcv_func)(struct sk_buff *, struct net_device *,
@@ -319,11 +319,12 @@ void caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
enum cfcnfg_phy_preference pref;
struct cfcnfg *cfg = get_cfcnfg(dev_net(dev));
struct caif_device_entry_list *caifdevs;
+ int res;
caifdevs = caif_device_list(dev_net(dev));
caifd = caif_device_alloc(dev);
if (!caifd)
- return;
+ return -ENOMEM;
*layer = &caifd->layer;
spin_lock_init(&caifd->flow_lock);
@@ -344,7 +345,7 @@ void caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
strlcpy(caifd->layer.name, dev->name,
sizeof(caifd->layer.name));
caifd->layer.transmit = transmit;
- cfcnfg_add_phy_layer(cfg,
+ res = cfcnfg_add_phy_layer(cfg,
dev,
&caifd->layer,
pref,
@@ -354,6 +355,7 @@ void caif_enroll_dev(struct net_device *dev, struct caif_dev_common *caifdev,
mutex_unlock(&caifdevs->lock);
if (rcv_func)
*rcv_func = receive;
+ return res;
}
EXPORT_SYMBOL(caif_enroll_dev);
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index 399239a14420..cac30e676ac9 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -450,7 +450,7 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
rcu_read_unlock();
}
-void
+int
cfcnfg_add_phy_layer(struct cfcnfg *cnfg,
struct net_device *dev, struct cflayer *phy_layer,
enum cfcnfg_phy_preference pref,
@@ -459,7 +459,7 @@ cfcnfg_add_phy_layer(struct cfcnfg *cnfg,
{
struct cflayer *frml;
struct cfcnfg_phyinfo *phyinfo = NULL;
- int i;
+ int i, res = 0;
u8 phyid;
mutex_lock(&cnfg->lock);
@@ -473,12 +473,15 @@ cfcnfg_add_phy_layer(struct cfcnfg *cnfg,
goto got_phyid;
}
pr_warn("Too many CAIF Link Layers (max 6)\n");
+ res = -EEXIST;
goto out;
got_phyid:
phyinfo = kzalloc(sizeof(struct cfcnfg_phyinfo), GFP_ATOMIC);
- if (!phyinfo)
+ if (!phyinfo) {
+ res = -ENOMEM;
goto out_err;
+ }
phy_layer->id = phyid;
phyinfo->pref = pref;
@@ -492,8 +495,10 @@ cfcnfg_add_phy_layer(struct cfcnfg *cnfg,
frml = cffrml_create(phyid, fcs);
- if (!frml)
+ if (!frml) {
+ res = -ENOMEM;
goto out_err;
+ }
phyinfo->frm_layer = frml;
layer_set_up(frml, cnfg->mux);
@@ -511,11 +516,12 @@ cfcnfg_add_phy_layer(struct cfcnfg *cnfg,
list_add_rcu(&phyinfo->node, &cnfg->phys);
out:
mutex_unlock(&cnfg->lock);
- return;
+ return res;
out_err:
kfree(phyinfo);
mutex_unlock(&cnfg->lock);
+ return res;
}
EXPORT_SYMBOL(cfcnfg_add_phy_layer);
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] net: caif: fix memory leak in caif_device_notify
2021-06-03 16:37 [PATCH 0/4] net: caif: fix 2 memory leaks Pavel Skripkin
2021-06-03 16:38 ` [PATCH 1/4] net: caif: added cfserl_release function Pavel Skripkin
2021-06-03 16:38 ` [PATCH 2/4] net: caif: add proper error handling Pavel Skripkin
@ 2021-06-03 16:39 ` Pavel Skripkin
2021-06-03 16:42 ` Pavel Skripkin
2021-06-03 16:39 ` [PATCH 4/4] net: caif: fix memory leak in cfusbl_device_notify Pavel Skripkin
2021-06-03 22:20 ` [PATCH 0/4] net: caif: fix 2 memory leaks patchwork-bot+netdevbpf
4 siblings, 1 reply; 7+ messages in thread
From: Pavel Skripkin @ 2021-06-03 16:39 UTC (permalink / raw)
To: davem, kuba, sjur.brandeland
Cc: netdev, linux-kernel, Pavel Skripkin, stable,
syzbot+7ec324747ce876a29db6
In case of caif_enroll_dev() fail, allocated
link_support won't be assigned to the corresponding
structure. So simply free allocated pointer in case
of error
Fixes: 7c18d2205ea7 ("caif: Restructure how link caif link layer enroll")
Cc: stable@vger.kernel.org
Reported-and-tested-by: syzbot+7ec324747ce876a29db6@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
net/caif/caif_dev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index fffbe41440b3..440139706130 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -370,6 +370,7 @@ static int caif_device_notify(struct notifier_block *me, unsigned long what,
struct cflayer *layer, *link_support;
int head_room = 0;
struct caif_device_entry_list *caifdevs;
+ int res;
cfg = get_cfcnfg(dev_net(dev));
caifdevs = caif_device_list(dev_net(dev));
@@ -395,8 +396,10 @@ static int caif_device_notify(struct notifier_block *me, unsigned long what,
break;
}
}
- caif_enroll_dev(dev, caifdev, link_support, head_room,
+ res = caif_enroll_dev(dev, caifdev, link_support, head_room,
&layer, NULL);
+ if (res)
+ cfserl_release(link_support);
caifdev->flowctrl = dev_flowctrl;
break;
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] net: caif: fix memory leak in caif_device_notify
2021-06-03 16:39 ` [PATCH 3/4] net: caif: fix memory leak in caif_device_notify Pavel Skripkin
@ 2021-06-03 16:42 ` Pavel Skripkin
0 siblings, 0 replies; 7+ messages in thread
From: Pavel Skripkin @ 2021-06-03 16:42 UTC (permalink / raw)
To: davem, kuba, sjur.brandeland
Cc: netdev, linux-kernel, stable, syzbot+7ec324747ce876a29db6
On Thu, 3 Jun 2021 19:39:11 +0300
Pavel Skripkin <paskripkin@gmail.com> wrote:
> In case of caif_enroll_dev() fail, allocated
> link_support won't be assigned to the corresponding
> structure. So simply free allocated pointer in case
> of error
>
> Fixes: 7c18d2205ea7 ("caif: Restructure how link caif link layer
> enroll") Cc: stable@vger.kernel.org
> Reported-and-tested-by:
> syzbot+7ec324747ce876a29db6@syzkaller.appspotmail.com Signed-off-by:
> Pavel Skripkin <paskripkin@gmail.com> ---
> net/caif/caif_dev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
> index fffbe41440b3..440139706130 100644
> --- a/net/caif/caif_dev.c
> +++ b/net/caif/caif_dev.c
> @@ -370,6 +370,7 @@ static int caif_device_notify(struct
> notifier_block *me, unsigned long what, struct cflayer *layer,
> *link_support; int head_room = 0;
> struct caif_device_entry_list *caifdevs;
> + int res;
>
> cfg = get_cfcnfg(dev_net(dev));
> caifdevs = caif_device_list(dev_net(dev));
> @@ -395,8 +396,10 @@ static int caif_device_notify(struct
> notifier_block *me, unsigned long what, break;
> }
> }
> - caif_enroll_dev(dev, caifdev, link_support,
> head_room,
> + res = caif_enroll_dev(dev, caifdev, link_support,
> head_room, &layer, NULL);
> + if (res)
> + cfserl_release(link_support);
> caifdev->flowctrl = dev_flowctrl;
> break;
>
One thing Im wondering about is should I return this error
from caif_device_notify()? I look forward to hearing your perspective on
this question and patch series :)
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 4/4] net: caif: fix memory leak in cfusbl_device_notify
2021-06-03 16:37 [PATCH 0/4] net: caif: fix 2 memory leaks Pavel Skripkin
` (2 preceding siblings ...)
2021-06-03 16:39 ` [PATCH 3/4] net: caif: fix memory leak in caif_device_notify Pavel Skripkin
@ 2021-06-03 16:39 ` Pavel Skripkin
2021-06-03 22:20 ` [PATCH 0/4] net: caif: fix 2 memory leaks patchwork-bot+netdevbpf
4 siblings, 0 replies; 7+ messages in thread
From: Pavel Skripkin @ 2021-06-03 16:39 UTC (permalink / raw)
To: davem, kuba, sjur.brandeland; +Cc: netdev, linux-kernel, Pavel Skripkin, stable
In case of caif_enroll_dev() fail, allocated
link_support won't be assigned to the corresponding
structure. So simply free allocated pointer in case
of error.
Fixes: 7ad65bf68d70 ("caif: Add support for CAIF over CDC NCM USB interface")
Cc: stable@vger.kernel.org
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
net/caif/caif_usb.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/net/caif/caif_usb.c b/net/caif/caif_usb.c
index a0116b9503d9..b02e1292f7f1 100644
--- a/net/caif/caif_usb.c
+++ b/net/caif/caif_usb.c
@@ -115,6 +115,11 @@ static struct cflayer *cfusbl_create(int phyid, u8 ethaddr[ETH_ALEN],
return (struct cflayer *) this;
}
+static void cfusbl_release(struct cflayer *layer)
+{
+ kfree(layer);
+}
+
static struct packet_type caif_usb_type __read_mostly = {
.type = cpu_to_be16(ETH_P_802_EX1),
};
@@ -127,6 +132,7 @@ static int cfusbl_device_notify(struct notifier_block *me, unsigned long what,
struct cflayer *layer, *link_support;
struct usbnet *usbnet;
struct usb_device *usbdev;
+ int res;
/* Check whether we have a NCM device, and find its VID/PID. */
if (!(dev->dev.parent && dev->dev.parent->driver &&
@@ -169,8 +175,11 @@ static int cfusbl_device_notify(struct notifier_block *me, unsigned long what,
if (dev->num_tx_queues > 1)
pr_warn("USB device uses more than one tx queue\n");
- caif_enroll_dev(dev, &common, link_support, CFUSB_MAX_HEADLEN,
+ res = caif_enroll_dev(dev, &common, link_support, CFUSB_MAX_HEADLEN,
&layer, &caif_usb_type.func);
+ if (res)
+ goto err;
+
if (!pack_added)
dev_add_pack(&caif_usb_type);
pack_added = true;
@@ -178,6 +187,9 @@ static int cfusbl_device_notify(struct notifier_block *me, unsigned long what,
strlcpy(layer->name, dev->name, sizeof(layer->name));
return 0;
+err:
+ cfusbl_release(link_support);
+ return res;
}
static struct notifier_block caif_device_notifier = {
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/4] net: caif: fix 2 memory leaks
2021-06-03 16:37 [PATCH 0/4] net: caif: fix 2 memory leaks Pavel Skripkin
` (3 preceding siblings ...)
2021-06-03 16:39 ` [PATCH 4/4] net: caif: fix memory leak in cfusbl_device_notify Pavel Skripkin
@ 2021-06-03 22:20 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-06-03 22:20 UTC (permalink / raw)
To: Pavel Skripkin; +Cc: davem, kuba, sjur.brandeland, netdev, linux-kernel
Hello:
This series was applied to netdev/net.git (refs/heads/master):
On Thu, 3 Jun 2021 19:37:27 +0300 you wrote:
> This patch series fix 2 memory leaks in caif
> interface.
>
> Syzbot reported memory leak in cfserl_create().
> The problem was in cfcnfg_add_phy_layer() function.
> This function accepts struct cflayer *link_support and
> assign it to corresponting structures, but it can fail
> in some cases.
>
> [...]
Here is the summary with links:
- [1/4] net: caif: added cfserl_release function
https://git.kernel.org/netdev/net/c/bce130e7f392
- [2/4] net: caif: add proper error handling
https://git.kernel.org/netdev/net/c/a2805dca5107
- [3/4] net: caif: fix memory leak in caif_device_notify
https://git.kernel.org/netdev/net/c/b53558a950a8
- [4/4] net: caif: fix memory leak in cfusbl_device_notify
https://git.kernel.org/netdev/net/c/7f5d86669fa4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread