linux-kernel - [PATCH 4.20 47/65] stm class: Fix a module refcount leak in policy creation error path

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190111131102.760686276@linuxfoundation.org>
Date:   Fri, 11 Jan 2019 15:15:33 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Ammy Yi <ammy.yi@...el.com>
Subject: [PATCH 4.20 47/65] stm class: Fix a module refcount leak in policy creation error path

4.20-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Alexander Shishkin <alexander.shishkin@...ux.intel.com>

commit c18614a1a11276837bdd44403d84d207c9951538 upstream.

Commit c7fd62bc69d0 ("stm class: Introduce framing protocol drivers")
adds a bug into the error path of policy creation, that would do a
module_put() on a wrong module, if one tried to create a policy for
an stm device which already has a policy, using a different protocol.
IOW,

| mkdir /config/stp-policy/dummy_stm.0:p_basic.test
| mkdir /config/stp-policy/dummy_stm.0:p_sys-t.test # puts "p_basic"
| mkdir /config/stp-policy/dummy_stm.0:p_sys-t.test # "p_basic" -> -1

throws:

| general protection fault: 0000 [#1] SMP PTI
| CPU: 3 PID: 2887 Comm: mkdir
| RIP: 0010:module_put.part.31+0xe/0x90
| Call Trace:
|  module_put+0x13/0x20
|  stm_put_protocol+0x11/0x20 [stm_core]
|  stp_policy_make+0xf1/0x210 [stm_core]
|  ? __kmalloc+0x183/0x220
|  ? configfs_mkdir+0x10d/0x4c0
|  configfs_mkdir+0x169/0x4c0
|  vfs_mkdir+0x108/0x1c0
|  do_mkdirat+0xe8/0x110
|  __x64_sys_mkdir+0x1b/0x20
|  do_syscall_64+0x5a/0x140
|  entry_SYSCALL_64_after_hwframe+0x44/0xa9

Correct this sad mistake by calling calling 'put' on the correct
reference, which happens to match another error path in the same
function, so we consolidate the two at the same time.

Signed-off-by: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Fixes: c7fd62bc69d0 ("stm class: Introduce framing protocol drivers")
Reported-by: Ammy Yi <ammy.yi@...el.com>
Cc: stable <stable@...r.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

---
 drivers/hwtracing/stm/policy.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

--- a/drivers/hwtracing/stm/policy.c
+++ b/drivers/hwtracing/stm/policy.c
@@ -440,10 +440,8 @@ stp_policy_make(struct config_group *gro
 
 	stm->policy = kzalloc(sizeof(*stm->policy), GFP_KERNEL);
 	if (!stm->policy) {
-		mutex_unlock(&stm->policy_mutex);
-		stm_put_protocol(pdrv);
-		stm_put_device(stm);
-		return ERR_PTR(-ENOMEM);
+		ret = ERR_PTR(-ENOMEM);
+		goto unlock_policy;
 	}
 
 	config_group_init_type_name(&stm->policy->group, name,
@@ -458,7 +456,11 @@ unlock_policy:
 	mutex_unlock(&stm->policy_mutex);
 
 	if (IS_ERR(ret)) {
-		stm_put_protocol(stm->pdrv);
+		/*
+		 * pdrv and stm->pdrv at this point can be quite different,
+		 * and only one of them needs to be 'put'
+		 */
+		stm_put_protocol(pdrv);
 		stm_put_device(stm);
 	}