Skip to content

net: caif: fix 2 memory leaks

Pavel Skripkin lkml netdev stable 1 version Applied

Status from this message: This series was applied to netdev/net.git (refs/heads/master):

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest4 patches 1 replyApplied 4/4

Patches in v1 All 4 applied

#Subject and filesSizeRepliesApplied
1/4net: caif: added cfserl_release functioninclude/net/caif/cfserl.h net/caif/cfserl.c+6 -00 repliesApplied
2/4net: caif: add proper error handlinginclude/net/caif/caif_dev.h include/net/caif/cfcnfg.h net/caif/caif_dev.c +1 more+18 -100 repliesApplied
3/4net: caif: fix memory leak in caif_device_notifynet/caif/caif_dev.c+4 -11 replyApplied
4/4net: caif: fix memory leak in cfusbl_device_notifynet/caif/caif_usb.c+13 -10 repliesApplied

Reviews trailers per patch

None of the 4 patches has a Reviewed-by or Acked-by.

Patch[email protected]
1/4
2/4
3/4Reported-and-tested-by
4/4

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v1 thread

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

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(-)
Signature · 1 line
2.31.1