[PATCH 0/4] net: caif: fix 2 memory leaks

* [PATCH 0/4] net: caif: fix 2 memory leaks
@ 2021-06-03 16:37 Pavel Skripkin
  2021-06-03 16:38 ` [PATCH 1/4] net: caif: added cfserl_release function Pavel Skripkin
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Pavel Skripkin @ 2021-06-03 16:37 UTC (permalink / raw)
  To: davem, kuba, sjur.brandeland; +Cc: netdev, linux-kernel, Pavel Skripkin

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.

These cases must be handled to prevent leaking allocated
struct cflayer *link_support pointer, because if error accured
before assigning link_support pointer to somewhere, this pointer
must be freed.

Fail log:

[   49.051872][ T7010] caif:cfcnfg_add_phy_layer(): Too many CAIF Link Layers (max 6)
[   49.110236][ T7042] caif:cfcnfg_add_phy_layer(): Too many CAIF Link Layers (max 6)
[   49.134936][ T7045] caif:cfcnfg_add_phy_layer(): Too many CAIF Link Layers (max 6)
[   49.163083][ T7043] caif:cfcnfg_add_phy_layer(): Too many CAIF Link Layers (max 6)
[   55.248950][ T6994] kmemleak: 4 new suspected memory leaks (see /sys/kernel/debug/kmemleak)

int cfcnfg_add_phy_layer(..., struct cflayer *link_support, ...)
{
...
	/* CAIF protocol allow maximum 6 link-layers */
	for (i = 0; i < 7; i++) {
		phyid = (dev->ifindex + i) & 0x7;
		if (phyid == 0)
			continue;
		if (cfcnfg_get_phyinfo_rcu(cnfg, phyid) == NULL)
			goto got_phyid;
	}
	pr_warn("Too many CAIF Link Layers (max 6)\n");
	goto out;
...
	if (link_support != NULL) {
		link_support->id = phyid;
		layer_set_dn(frml, link_support);
		layer_set_up(link_support, frml);
		layer_set_dn(link_support, phy_layer);
		layer_set_up(phy_layer, link_support);
	}
...
}

As you can see, if cfcnfg_add_phy_layer fails before layer_set_*,
link_support becomes leaked.

So, in this series, I made cfcnfg_add_phy_layer() 
return an int and added error handling code to prevent
leaking link_support pointer in caif_device_notify()
and cfusbl_device_notify() functions.

NOTE: this series was tested by syzbot
https://syzkaller.appspot.com/bug?id=62bc71b5fa73349e2e6b6280eca9c9615ddeb585)

Pavel Skripkin (4):
  net: caif: added cfserl_release function
  net: caif: add proper error handling
  net: caif: fix memory leak in caif_device_notify
  net: caif: fix memory leak in cfusbl_device_notify

 include/net/caif/caif_dev.h |  2 +-
 include/net/caif/cfcnfg.h   |  2 +-
 include/net/caif/cfserl.h   |  1 +
 net/caif/caif_dev.c         | 13 +++++++++----
 net/caif/caif_usb.c         | 14 +++++++++++++-
 net/caif/cfcnfg.c           | 16 +++++++++++-----
 net/caif/cfserl.c           |  5 +++++
 7 files changed, 41 insertions(+), 12 deletions(-)

-- 
2.31.1

^ permalink raw reply	[flat|nested] 7+ messages in thread