Please apply these fixes to the Tpm driver. I am resubmitting the entire
patch set that was orginally sent to LKML on April 27 with the changes
that were requested fixed.
Thanks,
Kylie
On Wed, 9 Mar 2005, Jeff Garzik wrote:
> Greg KH wrote:
<snip>
> > +
> > + down(&chip->timer_manipulation_mutex);
> > + chip->time_expired = 0;
> > + init_timer(&chip->device_timer);
> > + chip->device_timer.function = tpm_time_expired;
> > + chip->device_timer.expires = jiffies + 2 * 60 * HZ;
> > + chip->device_timer.data = (unsigned long) &chip->time_expired;
> > + add_timer(&chip->device_timer);
>
> very wrong. you init_timer() when you initialize 'chip'... once. then during
> the device lifetime you add/mod/del the timer.
>
> calling init_timer() could lead to corruption of state.
>
<snip>
> > + /* Set a timeout by which the reader must come claim the result */
> > + down(&chip->timer_manipulation_mutex);
> > + init_timer(&chip->user_read_timer);
> > + chip->user_read_timer.function = user_reader_timeout;
> > + chip->user_read_timer.data = (unsigned long) chip;
> > + chip->user_read_timer.expires = jiffies + (60 * HZ);
> > + add_timer(&chip->user_read_timer);
>
> again, don't repeatedly init_timer()
<snip>
> > +int tpm_register_hardware(struct pci_dev *pci_dev,
> > + struct tpm_vendor_specific *entry)
> > +{
> > + char devname[7];
> > + struct tpm_chip *chip;
> > + int i, j;
> > +
> > + /* Driver specific per-device data */
> > + chip = kmalloc(sizeof(*chip), GFP_KERNEL);
> > + if (chip == NULL)
> > + return -ENOMEM;
> > +
> > + memset(chip, 0, sizeof(struct tpm_chip));
> > +
> > + init_MUTEX(&chip->buffer_mutex);
> > + init_MUTEX(&chip->tpm_mutex);
> > + init_MUTEX(&chip->timer_manipulation_mutex);
> > + INIT_LIST_HEAD(&chip->list);
>
> timer init should be here
<snip>
Fix the timer to be inited and modified properly. This work depends on
the fixing of the msleep stuff which was submitted in a patch by Nish
Aravamudan on March 10.
Signed-of-by: Kylene Hall <[email protected]>
---
diff -urpN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.c linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.c 2005-04-25 18:49:08.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.c 2005-04-26 12:47:20.000000000 -0500
@@ -456,16 +456,7 @@ int tpm_release(struct inode *inode, str
spin_lock(&driver_lock);
chip->num_opens--;
- spin_unlock(&driver_lock);
-
- down(&chip->timer_manipulation_mutex);
- if (timer_pending(&chip->user_read_timer))
- del_singleshot_timer_sync(&chip->user_read_timer);
- else if (timer_pending(&chip->device_timer))
- del_singleshot_timer_sync(&chip->device_timer);
- up(&chip->timer_manipulation_mutex);
-
- kfree(chip->data_buffer);
+ del_singleshot_timer_sync(&chip->user_read_timer);
atomic_set(&chip->data_pending, 0);
pci_dev_put(chip->pci_dev);
@@ -504,13 +495,7 @@ tpm_write(struct file * file, const char
up(&chip->buffer_mutex);
/* Set a timeout by which the reader must come claim the result */
- down(&chip->timer_manipulation_mutex);
- init_timer(&chip->user_read_timer);
- chip->user_read_timer.function = user_reader_timeout;
- chip->user_read_timer.data = (unsigned long) chip;
- chip->user_read_timer.expires = jiffies + (60 * HZ);
- add_timer(&chip->user_read_timer);
- up(&chip->timer_manipulation_mutex);
+ mod_timer(&chip->user_read_timer, jiffies + (60 * HZ));
return in_size;
}
@@ -639,9 +624,12 @@ int tpm_register_hardware(struct pci_dev
init_MUTEX(&chip->buffer_mutex);
init_MUTEX(&chip->tpm_mutex);
- init_MUTEX(&chip->timer_manipulation_mutex);
INIT_LIST_HEAD(&chip->list);
+ init_timer(&chip->user_read_timer);
+ chip->user_read_timer.function = user_reader_timeout;
+ chip->user_read_timer.data = (unsigned long) chip;
+
chip->vendor = entry;
chip->dev_num = -1;
diff -urpN --exclude='*.orig' linux-2.6.12-rc2/drivers/char/tpm/tpm.h linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.h
--- linux-2.6.12-rc2/drivers/char/tpm/tpm.h 2005-04-25 18:49:08.000000000 -0500
+++ linux-2.6.12-rc2-tpmdd/drivers/char/tpm/tpm.h 2005-04-26 12:53:29.000000000 -0500
@@ -76,8 +76,6 @@ struct tpm_chip {
struct timer_list user_read_timer; /* user needs to claim result */
struct semaphore tpm_mutex; /* tpm is processing */
- struct timer_list device_timer; /* tpm is processing */
- struct semaphore timer_manipulation_mutex;
struct tpm_vendor_specific *vendor;