All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
@ 2019-03-27 18:32 Tadeusz Struk
  2019-03-28 12:34 ` Jarkko Sakkinen
  0 siblings, 1 reply; 9+ messages in thread
From: Tadeusz Struk @ 2019-03-27 18:32 UTC (permalink / raw)
  To: jarkko.sakkinen
  Cc: grawity, James.Bottomley, linux-integrity, linux-kernel, stable,
	tadeusz.struk

The poll condition should only check response_length,
because reads should only be issued if there is data to read.
The response_read flag only prevents double writes.
The problem was that the write set the response_read to false,
enqued a tpm job, and returned. Then application called poll
which checked the response_read flag and returned EPOLLIN.
Then the application called read, but got nothing.
After all that the async_work kicked in.
Added also mutex_lock around the poll check to prevent
other possible race conditions.

Cc: stable@vger.kernel.org
Fixes: 9488585b21bef0df12 ("tpm: add support for partial reads")
Reported-by: Mantas Mikulėnas <grawity@gmail.com>
Tested-by: Mantas Mikulėnas <grawity@gmail.com>
Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
---
 drivers/char/tpm/tpm-dev-common.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 5eecad233ea1..744b0237300a 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -203,12 +203,19 @@ __poll_t tpm_common_poll(struct file *file, poll_table *wait)
 	__poll_t mask = 0;
 
 	poll_wait(file, &priv->async_wait, wait);
+	mutex_lock(&priv->buffer_mutex);
 
-	if (!priv->response_read || priv->response_length)
+	/*
+	 * The response_length indicates if there is still response
+	 * (or part of it) to be consumed. Partial reads decrease it
+	 * by the number of bytes read, and write resets it the zero.
+	 */
+	if (priv->response_length)
 		mask = EPOLLIN | EPOLLRDNORM;
 	else
 		mask = EPOLLOUT | EPOLLWRNORM;
 
+	mutex_unlock(&priv->buffer_mutex);
 	return mask;
 }
 


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

* Re: [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
  2019-03-27 18:32 [PATCH v4] tpm: fix an invalid condition in tpm_common_poll Tadeusz Struk
@ 2019-03-28 12:34 ` Jarkko Sakkinen
  2019-03-28 16:34   ` Tadeusz Struk
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2019-03-28 12:34 UTC (permalink / raw)
  To: Tadeusz Struk
  Cc: grawity, James.Bottomley, linux-integrity, linux-kernel, stable

On Wed, Mar 27, 2019 at 11:32:38AM -0700, Tadeusz Struk wrote:
> The poll condition should only check response_length,
> because reads should only be issued if there is data to read.
> The response_read flag only prevents double writes.
> The problem was that the write set the response_read to false,
> enqued a tpm job, and returned. Then application called poll
> which checked the response_read flag and returned EPOLLIN.
> Then the application called read, but got nothing.
> After all that the async_work kicked in.
> Added also mutex_lock around the poll check to prevent
> other possible race conditions.
> 
> Cc: stable@vger.kernel.org
> Fixes: 9488585b21bef0df12 ("tpm: add support for partial reads")
> Reported-by: Mantas Mikulėnas <grawity@gmail.com>
> Tested-by: Mantas Mikulėnas <grawity@gmail.com>
> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Thank you, it is applied.

/Jarkko

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

* Re: [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
  2019-03-28 12:34 ` Jarkko Sakkinen
@ 2019-03-28 16:34   ` Tadeusz Struk
  2019-04-08 12:01     ` Thibaut Sautereau
  0 siblings, 1 reply; 9+ messages in thread
From: Tadeusz Struk @ 2019-03-28 16:34 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: grawity, James.Bottomley, linux-integrity, linux-kernel, stable

On 3/28/19 5:34 AM, Jarkko Sakkinen wrote:
> Thank you, it is applied.

Thank you Jarkko.

-- 
Tadeusz

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

* Re: [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
  2019-03-28 16:34   ` Tadeusz Struk
@ 2019-04-08 12:01     ` Thibaut Sautereau
  2019-04-09 13:44       ` Jarkko Sakkinen
  0 siblings, 1 reply; 9+ messages in thread
From: Thibaut Sautereau @ 2019-04-08 12:01 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Tadeusz Struk, grawity, James.Bottomley, linux-integrity,
	linux-kernel, stable

Hello Jarkko,

On Thu, Mar 28, 2019 at 09:34:18AM -0700, Tadeusz Struk wrote:
> On 3/28/19 5:34 AM, Jarkko Sakkinen wrote:
> > Thank you, it is applied.
> 
> Thank you Jarkko.

What's the status of this patch now? It's needed in linux-5.0.y as TPM
2.0 support is currently broken with those stable kernels without this
commit.

Thanks,

-- 
Thibaut

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

* Re: [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
  2019-04-08 12:01     ` Thibaut Sautereau
@ 2019-04-09 13:44       ` Jarkko Sakkinen
  2019-04-23 20:54         ` Jonas Witschel
  0 siblings, 1 reply; 9+ messages in thread
From: Jarkko Sakkinen @ 2019-04-09 13:44 UTC (permalink / raw)
  To: Thibaut Sautereau
  Cc: Tadeusz Struk, grawity, James.Bottomley, linux-integrity,
	linux-kernel, stable

On Mon, Apr 08, 2019 at 02:01:38PM +0200, Thibaut Sautereau wrote:
> Hello Jarkko,
> 
> On Thu, Mar 28, 2019 at 09:34:18AM -0700, Tadeusz Struk wrote:
> > On 3/28/19 5:34 AM, Jarkko Sakkinen wrote:
> > > Thank you, it is applied.
> > 
> > Thank you Jarkko.
> 
> What's the status of this patch now? It's needed in linux-5.0.y as TPM
> 2.0 support is currently broken with those stable kernels without this
> commit.

part of a PR.

https://lore.kernel.org/linux-integrity/20190329115544.GA27351@linux.intel.com/

/Jarkko

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

* Re: [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
  2019-04-09 13:44       ` Jarkko Sakkinen
@ 2019-04-23 20:54         ` Jonas Witschel
  2019-04-24  0:43           ` Sasha Levin
  2019-05-02  7:22           ` Jarkko Sakkinen
  0 siblings, 2 replies; 9+ messages in thread
From: Jonas Witschel @ 2019-04-23 20:54 UTC (permalink / raw)
  To: Jarkko Sakkinen, Thibaut Sautereau
  Cc: Tadeusz Struk, grawity, James.Bottomley, linux-integrity,
	linux-kernel, stable, Jonas Witschel


[-- Attachment #1.1: Type: text/plain, Size: 932 bytes --]

On 2019-04-09 15:44, Jarkko Sakkinen wrote:
> On Mon, Apr 08, 2019 at 02:01:38PM +0200, Thibaut Sautereau wrote:
>> [...]
>> What's the status of this patch now? It's needed in linux-5.0.y as TPM
>> 2.0 support is currently broken with those stable kernels without this
>> commit.
> 
> part of a PR.
> 
> https://lore.kernel.org/linux-integrity/20190329115544.GA27351@linux.intel.com/

It appears that the final version of the patch that was merged to
Linus's tree [1] does not include the "Cc: stable@vger.kernel.org" tag.
If I understand correctly, this means that the patch will not be
automatically included in the -stable tree without further action. Is
there a specific reason not to apply this patch to 5.0.x, or did the tag
just get lost in the merge process?

Cheers,
Jonas

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7110629263469b4664d00b38ef80a656eddf3637


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
  2019-04-23 20:54         ` Jonas Witschel
@ 2019-04-24  0:43           ` Sasha Levin
  2019-04-24  7:06             ` Jonas Witschel
  2019-05-02  7:22           ` Jarkko Sakkinen
  1 sibling, 1 reply; 9+ messages in thread
From: Sasha Levin @ 2019-04-24  0:43 UTC (permalink / raw)
  To: Jonas Witschel
  Cc: Jarkko Sakkinen, Thibaut Sautereau, Tadeusz Struk, grawity,
	James.Bottomley, linux-integrity, linux-kernel, stable

On Tue, Apr 23, 2019 at 10:54:47PM +0200, Jonas Witschel wrote:
>On 2019-04-09 15:44, Jarkko Sakkinen wrote:
>> On Mon, Apr 08, 2019 at 02:01:38PM +0200, Thibaut Sautereau wrote:
>>> [...]
>>> What's the status of this patch now? It's needed in linux-5.0.y as TPM
>>> 2.0 support is currently broken with those stable kernels without this
>>> commit.
>>
>> part of a PR.
>>
>> https://lore.kernel.org/linux-integrity/20190329115544.GA27351@linux.intel.com/
>
>It appears that the final version of the patch that was merged to
>Linus's tree [1] does not include the "Cc: stable@vger.kernel.org" tag.
>If I understand correctly, this means that the patch will not be
>automatically included in the -stable tree without further action. Is
>there a specific reason not to apply this patch to 5.0.x, or did the tag
>just get lost in the merge process?

Good catch; I see that Jarkko had the same comment on v3 but v4 ended up
being without the -stable tag without any explanation. I've queued this
for 5.0, it doesn't seem relevant for older branches.

--
Thanks,
Sasha

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

* Re: [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
  2019-04-24  0:43           ` Sasha Levin
@ 2019-04-24  7:06             ` Jonas Witschel
  0 siblings, 0 replies; 9+ messages in thread
From: Jonas Witschel @ 2019-04-24  7:06 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Jarkko Sakkinen, Thibaut Sautereau, Tadeusz Struk, grawity,
	James.Bottomley, linux-integrity, linux-kernel, stable


[-- Attachment #1.1: Type: text/plain, Size: 1226 bytes --]

On 2019-04-24 02:43, Sasha Levin wrote:
> On Tue, Apr 23, 2019 at 10:54:47PM +0200, Jonas Witschel wrote:
>> On 2019-04-09 15:44, Jarkko Sakkinen wrote:
>>> On Mon, Apr 08, 2019 at 02:01:38PM +0200, Thibaut Sautereau wrote:
>>>> [...]
>>>> What's the status of this patch now? It's needed in linux-5.0.y as TPM
>>>> 2.0 support is currently broken with those stable kernels without this
>>>> commit.
>>>
>>> part of a PR.
>>>
>>> https://lore.kernel.org/linux-integrity/20190329115544.GA27351@linux.intel.com/
>>>
>>
>> It appears that the final version of the patch that was merged to
>> Linus's tree [1] does not include the "Cc: stable@vger.kernel.org" tag.
>> If I understand correctly, this means that the patch will not be
>> automatically included in the -stable tree without further action. Is
>> there a specific reason not to apply this patch to 5.0.x, or did the tag
>> just get lost in the merge process?
> 
> Good catch; I see that Jarkko had the same comment on v3 but v4 ended up
> being without the -stable tag without any explanation. I've queued this
> for 5.0, it doesn't seem relevant for older branches.

Thank you! Correct, the regression only affects 5.0.

Regards,
Jonas


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v4] tpm: fix an invalid condition in tpm_common_poll
  2019-04-23 20:54         ` Jonas Witschel
  2019-04-24  0:43           ` Sasha Levin
@ 2019-05-02  7:22           ` Jarkko Sakkinen
  1 sibling, 0 replies; 9+ messages in thread
From: Jarkko Sakkinen @ 2019-05-02  7:22 UTC (permalink / raw)
  To: Jonas Witschel
  Cc: Thibaut Sautereau, Tadeusz Struk, grawity, James.Bottomley,
	linux-integrity, linux-kernel, stable

On Tue, Apr 23, 2019 at 10:54:47PM +0200, Jonas Witschel wrote:
> On 2019-04-09 15:44, Jarkko Sakkinen wrote:
> > On Mon, Apr 08, 2019 at 02:01:38PM +0200, Thibaut Sautereau wrote:
> >> [...]
> >> What's the status of this patch now? It's needed in linux-5.0.y as TPM
> >> 2.0 support is currently broken with those stable kernels without this
> >> commit.
> > 
> > part of a PR.
> > 
> > https://lore.kernel.org/linux-integrity/20190329115544.GA27351@linux.intel.com/
> 
> It appears that the final version of the patch that was merged to
> Linus's tree [1] does not include the "Cc: stable@vger.kernel.org" tag.
> If I understand correctly, this means that the patch will not be
> automatically included in the -stable tree without further action. Is
> there a specific reason not to apply this patch to 5.0.x, or did the tag

It is my mistake. What I can do is to post it manually to stable.
I promise to do it as soon as it reaches the mainline.

/Jarkko

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

end of thread, other threads:[~2019-05-02  7:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-27 18:32 [PATCH v4] tpm: fix an invalid condition in tpm_common_poll Tadeusz Struk
2019-03-28 12:34 ` Jarkko Sakkinen
2019-03-28 16:34   ` Tadeusz Struk
2019-04-08 12:01     ` Thibaut Sautereau
2019-04-09 13:44       ` Jarkko Sakkinen
2019-04-23 20:54         ` Jonas Witschel
2019-04-24  0:43           ` Sasha Levin
2019-04-24  7:06             ` Jonas Witschel
2019-05-02  7:22           ` Jarkko Sakkinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.