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; }
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
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,
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.c...
/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/20190329115544.GA27351@linux.intel.c...
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/?i...
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.c...
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
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.c...
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
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.c...
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
linux-stable-mirror@lists.linaro.org