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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Thu,  3 Jun 2021 19:37:27 +0300
From:   Pavel Skripkin <paskripkin@...il.com>
To:     davem@...emloft.net, kuba@...nel.org,
        sjur.brandeland@...ricsson.com
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Pavel Skripkin <paskripkin@...il.com>
Subject: [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(-)

-- 
2.31.1