Patch to clean up a couple of issues recently pointed out with the
driver. The fix for issue (c) was also pointed out by Steve Tate on the
tpm-devel list.
On Thu, 2005-10-27 at 14:55 -0700, Andrew Morton wrote:
> Have just taken a wander through the TPM driver. I couldn't help myself - are
> you OK with the below trivial changes?
>
> Other things:
>
> a)
>
> ssize_t tpm_read(struct file * file, char __user *buf,
> size_t size, loff_t * off)
> {
> f struct tpm_chip *chip = file->private_data;
> int ret_size;
>
> del_singleshot_timer_sync(&chip->user_read_timer);
> 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;
>
> down(&chip->buffer_mutex);
> if (copy_to_user
> ((void __user *) buf, chip->data_buffer, ret_size))
> ret_size = -EFAULT;
>
> Why is the first arg to copy_to_user() typecast in this manner? I don't
> think the cast needs to be there?
>
Fixed in the patch below.
>
> b) user_reader_timeout does down() from within a timer handler! That's
> deadlocky and is illegal - timer handlers are run from interrupt
> context.
>
> This should have generated a storm of runtime warnings if tested with
> CONFIG_PREEMPT and CONFIG_DEBUG_SPINLOCK_SLEEP. Developers really should
> enable all the kernel debug options during development - they find bugs.
>
> Suggest you convert this to using schedule_work() or
> schedule_delayed_work().
>
I'll look into this.
> c) In tpm_remove_hardware():
>
> dev_mask[chip->dev_num / TPM_NUM_MASK_ENTRIES ] &= !(1 << (chip->dev_num % TPM_NUM_MASK_ENTRIES));
>
> Is that `!' supposed to be there? Looks odd.
>
Fixed in the patch below.
> d) How did this happen?
>
> int tpm_pm_suspend(struct device *dev, pm_message_t pm_state, u32 level)
>
> device_driver.suspend() only takes two arguments nowadays.
>
>
> e)
>
> int tpm_pm_resume(struct device *dev, u32 level)
>
> Ditto
>
>
Signed-off-by: Kylene Hall <[email protected]>
---
--- linux-2.6.14/drivers/char/tpm/tpm.c.orig 2005-11-11 14:08:48.000000000 -0600
+++ linux-2.6.14/drivers/char/tpm/tpm.c 2005-11-11 14:09:47.000000000
-0600
@@ -428,8 +428,7 @@ ssize_t tpm_read(struct file * file, cha
ret_size = size;
down(&chip->buffer_mutex);
- if (copy_to_user
- ((void __user *) buf, chip->data_buffer, ret_size))
+ if (copy_to_user(buf, chip->data_buffer, ret_size))
ret_size = -EFAULT;
up(&chip->buffer_mutex);
}
@@ -460,7 +459,7 @@ void tpm_remove_hardware(struct device *
sysfs_remove_group(&dev->kobj, chip->vendor->attr_group);
dev_mask[chip->dev_num / TPM_NUM_MASK_ENTRIES ] &=
- !(1 << (chip->dev_num % TPM_NUM_MASK_ENTRIES));
+ ~(1 << (chip->dev_num % TPM_NUM_MASK_ENTRIES));
kfree(chip);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
