[GIT PULL 0/3] stm class/intel_th: Fixes for v4.20

[GIT PULL 0/3] stm class/intel_th: Fixes for v4.20

[Alexander Shishkin]

Hi Greg,

I have 3 fixes for STM Class and Intel TH. Two things that I introduced in
v4.20-rc1 and one long-standing bug since v4.4. As usual, there is a signed
tag ready for pulling and individual patches follow. Please consider pulling
or applying. Thanks!

The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a:

  Linux 4.20-rc1 (2018-11-04 15:37:52 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ash/stm.git tags/intel_th-fixes-for-greg-20181214

for you to fetch changes up to ac83e602f95cd472062ea7d813086722fb59c496:

  intel_th: msu: Fix an off-by-one in attribute store (2018-12-14 17:39:33 +0200)

----------------------------------------------------------------
stm class/intel_th: Fixes for v4.20

These are:
 * module refcount leak in stm class, introduced in v4.20-rc1
 * documentation fix
 * string parsing off-by-one in intel_th, also for -stable

----------------------------------------------------------------
Alexander Shishkin (3):
      stm class: Fix a module refcount leak in policy creation error path
      stm class: Add a reference to the SyS-T document
      intel_th: msu: Fix an off-by-one in attribute store

 Documentation/trace/index.rst    |  1 +
 drivers/hwtracing/intel_th/msu.c |  3 ++-
 drivers/hwtracing/stm/policy.c   | 12 +++++++-----
 3 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.19.2

[GIT PULL 1/3] stm class: Fix a module refcount leak in policy creation error path

[Alexander Shishkin]

Commit ccddbbf4ef27 ("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@linux.intel.com>
Fixes: ccddbbf4ef27 ("stm class: Introduce framing protocol drivers")
Reported-by: Ammy Yi <ammy.yi@intel.com>
---
 drivers/hwtracing/stm/policy.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
index 0910ec807187..4b9e44b227d8 100644
--- a/drivers/hwtracing/stm/policy.c
+++ b/drivers/hwtracing/stm/policy.c
@@ -440,10 +440,8 @@ stp_policy_make(struct config_group *group, const char *name)
 
 	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 @@ stp_policy_make(struct config_group *group, const char *name)
 	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);
 	}
 
-- 
2.19.2

Re: [GIT PULL 1/3] stm class: Fix a module refcount leak in policy creation error path

[Greg Kroah-Hartman]

On Fri, Dec 14, 2018 at 05:53:45PM +0200, Alexander Shishkin wrote:
> Commit ccddbbf4ef27 ("stm class: Introduce framing protocol drivers")

I don't see that commit id in Linus's tree, are you sure it is correct?

thanks,

greg k-h

Re: [GIT PULL 1/3] stm class: Fix a module refcount leak in policy creation error path

[Alexander Shishkin]

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Fri, Dec 14, 2018 at 05:53:45PM +0200, Alexander Shishkin wrote:
>> Commit ccddbbf4ef27 ("stm class: Introduce framing protocol drivers")
>
> I don't see that commit id in Linus's tree, are you sure it is correct?

No, I messed it up somehow. Sorry about that. The real commit is
c7fd62bc69d0. Fixed version follows. Let me know if I should re-send the
whole thing.

Thanks,
--
Alex

Re: [GIT PULL 1/3] stm class: Fix a module refcount leak in policy creation error path

[Greg Kroah-Hartman]

On Wed, Dec 19, 2018 at 01:45:17PM +0200, Alexander Shishkin wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> 
> > On Fri, Dec 14, 2018 at 05:53:45PM +0200, Alexander Shishkin wrote:
> >> Commit ccddbbf4ef27 ("stm class: Introduce framing protocol drivers")
> >
> > I don't see that commit id in Linus's tree, are you sure it is correct?
> 
> No, I messed it up somehow. Sorry about that. The real commit is
> c7fd62bc69d0. Fixed version follows. Let me know if I should re-send the
> whole thing.

Please resend all of the patches.

thanks,

greg k-h

[PATCH] stm class: Fix a module refcount leak in policy creation error path

[Alexander Shishkin]

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@linux.intel.com>
Fixes: c7fd62bc69d0 ("stm class: Introduce framing protocol drivers")
Reported-by: Ammy Yi <ammy.yi@intel.com>
---
 drivers/hwtracing/stm/policy.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
index 0910ec807187..4b9e44b227d8 100644
--- a/drivers/hwtracing/stm/policy.c
+++ b/drivers/hwtracing/stm/policy.c
@@ -440,10 +440,8 @@ stp_policy_make(struct config_group *group, const char *name)
 
 	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 @@ stp_policy_make(struct config_group *group, const char *name)
 	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);
 	}
 
-- 
2.19.2

[GIT PULL 2/3] stm class: Add a reference to the SyS-T document

[Alexander Shishkin]

Commit 4cb3653df0cd ("stm class: Document the MIPI SyS-T protocol usage")
added a document describing the SyS-T protocol usage, but forgot to add
it to the directory index. Fix that.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Fixes: 4cb3653df0cd ("stm class: Document the MIPI SyS-T protocol usage")
---
 Documentation/trace/index.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/trace/index.rst b/Documentation/trace/index.rst
index 306997941ba1..6b4107cf4b98 100644
--- a/Documentation/trace/index.rst
+++ b/Documentation/trace/index.rst
@@ -22,3 +22,4 @@ Linux Tracing Technologies
    hwlat_detector
    intel_th
    stm
+   sys-t
-- 
2.19.2

[GIT PULL 3/3] intel_th: msu: Fix an off-by-one in attribute store

[Alexander Shishkin]

The 'nr_pages' attribute of the 'msc' subdevices parses a comma-separated
list of window sizes, passed from userspace. However, there is a bug in
the string parsing logic wherein it doesn't exclude the comma character
from the range of characters as it consumes them. This leads to an
out-of-bounds access given a sufficiently long list. For example:

> # echo 8,8,8,8 > /sys/bus/intel_th/devices/0-msc0/nr_pages
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in memchr+0x1e/0x40
> Read of size 1 at addr ffff8803ffcebcd1 by task sh/825
>
> CPU: 3 PID: 825 Comm: npktest.sh Tainted: G        W         4.20.0-rc1+
> Call Trace:
>  dump_stack+0x7c/0xc0
>  print_address_description+0x6c/0x23c
>  ? memchr+0x1e/0x40
>  kasan_report.cold.5+0x241/0x308
>  memchr+0x1e/0x40
>  nr_pages_store+0x203/0xd00 [intel_th_msu]

Fix this by accounting for the comma character.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Fixes: ba82664c134ef ("intel_th: Add Memory Storage Unit driver")
Cc: stable@vger.kernel.org # v4.4+
---
 drivers/hwtracing/intel_th/msu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index d293e55553bd..ba7aaf421f36 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -1423,7 +1423,8 @@ nr_pages_store(struct device *dev, struct device_attribute *attr,
 		if (!end)
 			break;
 
-		len -= end - p;
+		/* consume the number and the following comma, hence +1 */
+		len -= end - p + 1;
 		p = end + 1;
 	} while (len);
 
-- 
2.19.2