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: [email protected]
Fixes: 9488585b21bef0df12 ("tpm: add support for partial reads")
Reported-by: Mantas Mikulėnas <[email protected]>
Tested-by: Mantas Mikulėnas <[email protected]>
Signed-off-by: Tadeusz Struk <[email protected]>
---
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;
}
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: [email protected]
> Fixes: 9488585b21bef0df12 ("tpm: add support for partial reads")
> Reported-by: Mantas Mikulėnas <[email protected]>
> Tested-by: Mantas Mikulėnas <[email protected]>
> Signed-off-by: Tadeusz Struk <[email protected]>
Reviewed-by: Jarkko Sakkinen <[email protected]>
Thank you, it is applied.
/Jarkko
On 3/28/19 5:34 AM, Jarkko Sakkinen wrote:
> Thank you, it is applied.
Thank you Jarkko.
--
Tadeusz
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
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/[email protected]/
/Jarkko
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/[email protected]/
It appears that the final version of the patch that was merged to
Linus's tree [1] does not include the "Cc: [email protected]" 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
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/[email protected]/
>
>It appears that the final version of the patch that was merged to
>Linus's tree [1] does not include the "Cc: [email protected]" 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
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/[email protected]/
>>>
>>
>> It appears that the final version of the patch that was merged to
>> Linus's tree [1] does not include the "Cc: [email protected]" 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
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/[email protected]/
>
> It appears that the final version of the patch that was merged to
> Linus's tree [1] does not include the "Cc: [email protected]" 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