Re: [tpmdd-devel] [PATCH 2/3] TPM: Close data_pending anddata_buffer races

From: Mimi Zohar
Date: Tue Dec 27 2011 - 15:06:08 EST

On Fri, 2011-12-23 at 07:25 -0700, Tim Gardner wrote:
> On 12/22/2011 01:02 PM, Rajiv Andrade wrote:


> > It's inside the mutex region.
> >
> Actually, the patch you sent ( is
> _outside_ the mutex area, but I got your drift.

Yes, thanks for pointing it out in your original comments. I haven't
tested the following patch, but perhaps it will help clarify the updated
version, that I think Rajiv was describing ...

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 6a8771f..7dafd95 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -1210,7 +1210,6 @@ ssize_t tpm_read(struct file *file, char __user *buf,
ret_size = atomic_read(&chip->data_pending);
- atomic_set(&chip->data_pending, 0);
if (ret_size > 0) { /* relay data */
if (size < ret_size)
ret_size = size;
@@ -1221,8 +1220,10 @@ ssize_t tpm_read(struct file *file, char __user *buf,
if (rc)
ret_size = -EFAULT;

+ atomic_set(&chip->data_pending, 0);
- }
+ } else if (ret_size < 0)
+ atomic_set(&chip->data_pending, 0);

return ret_size;

> Even clearing chip->data_pending _inside_ the mutex area, I'm not sure
> it closes all of the race windows. I think its still possible to race
> with mod_timer() and del_singleshot_timer_sync(). Therein lies my point,
> if the exclusion code is not _obviously_ correct for something this
> simple, then its probably not. I think my patch is the correct approach.

With the above patch, tpm_read() resets the data_pending flag only after
copying the data to userspace, resolving the original race condition

> > This would require another fix though. tpm_write() doesn't check
> > tpm_transmit return code (and it should).
> > In case it returns an error (< 0), chip->data_pending would remain the
> > same forever with that change.
> >
> This observation is also correct, but not relevant to the exclusion
> races. It deserves a separate patch.

With the above patch, tpm_read() resets the data_pending flag only if
set, resolving the other race condition(s).



To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at