From 1631cfb7cee28388b04aef6c0a73050f6fd76e4d Mon Sep 17 00:00:00 2001 From: Gang Wei Date: Tue, 9 Oct 2012 17:35:22 +0800 Subject: driver/char/tpm: fix regression causesd by ppi This patch try to fix the S3 regression https://lkml.org/lkml/2012/10/5/433, which includes below line: [ 1554.684638] sysfs: cannot create duplicate filename '/devices/pnp0/00:0c/ppi' The root cause is that ppi sysfs teardown code is MIA, so while S3 resume, the ppi kobject will be created again upon existing one. To make the tear down code simple, change the ppi subfolder creation from using kobject_create_and_add to just using a named ppi attribute_group. Then ppi sysfs teardown could be done with a simple sysfs_remove_group call. Adjusted the name & return type for ppi sysfs init function. Reported-by: Ben Guthro Signed-off-by: Gang Wei Signed-off-by: Kent Yoder diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c index f26afdb..b429f1e 100644 --- a/drivers/char/tpm/tpm.c +++ b/drivers/char/tpm/tpm.c @@ -1259,6 +1259,7 @@ void tpm_remove_hardware(struct device *dev) misc_deregister(&chip->vendor.miscdev); sysfs_remove_group(&dev->kobj, chip->vendor.attr_group); + tpm_remove_ppi(&dev->kobj); tpm_bios_log_teardown(chip->bios_dir); /* write it this way to be explicit (chip->dev == dev) */ @@ -1476,7 +1477,7 @@ struct tpm_chip *tpm_register_hardware(struct device *dev, goto put_device; } - if (sys_add_ppi(&dev->kobj)) { + if (tpm_add_ppi(&dev->kobj)) { misc_deregister(&chip->vendor.miscdev); goto put_device; } diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 02c266a..8ef7649 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -329,10 +329,15 @@ extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long, wait_queue_head_t *); #ifdef CONFIG_ACPI -extern ssize_t sys_add_ppi(struct kobject *parent); +extern int tpm_add_ppi(struct kobject *); +extern void tpm_remove_ppi(struct kobject *); #else -static inline ssize_t sys_add_ppi(struct kobject *parent) +static inline int tpm_add_ppi(struct kobject *parent) { return 0; } + +static inline void tpm_remove_ppi(struct kobject *parent) +{ +} #endif diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c index f27b58c..720ebcf 100644 --- a/drivers/char/tpm/tpm_ppi.c +++ b/drivers/char/tpm/tpm_ppi.c @@ -444,18 +444,20 @@ static struct attribute *ppi_attrs[] = { &dev_attr_vs_operations.attr, NULL, }; static struct attribute_group ppi_attr_grp = { + .name = "ppi", .attrs = ppi_attrs }; -ssize_t sys_add_ppi(struct kobject *parent) +int tpm_add_ppi(struct kobject *parent) { - struct kobject *ppi; - ppi = kobject_create_and_add("ppi", parent); - if (sysfs_create_group(ppi, &ppi_attr_grp)) - return -EFAULT; - else - return 0; + return sysfs_create_group(parent, &ppi_attr_grp); +} +EXPORT_SYMBOL_GPL(tpm_add_ppi); + +void tpm_remove_ppi(struct kobject *parent) +{ + sysfs_remove_group(parent, &ppi_attr_grp); } -EXPORT_SYMBOL_GPL(sys_add_ppi); +EXPORT_SYMBOL_GPL(tpm_remove_ppi); MODULE_LICENSE("GPL"); -- cgit v0.10.2 From abce9ac292e13da367bbd22c1f7669f988d931ac Mon Sep 17 00:00:00 2001 From: Peter Huewe Date: Thu, 27 Sep 2012 16:09:33 +0200 Subject: tpm: Propagate error from tpm_transmit to fix a timeout hang tpm_write calls tpm_transmit without checking the return value and assigns the return value unconditionally to chip->pending_data, even if it's an error value. This causes three bugs. So if we write to /dev/tpm0 with a tpm_param_size bigger than TPM_BUFSIZE=0x1000 (e.g. 0x100a) and a bufsize also bigger than TPM_BUFSIZE (e.g. 0x100a) tpm_transmit returns -E2BIG which is assigned to chip->pending_data as -7, but tpm_write returns that TPM_BUFSIZE bytes have been successfully been written to the TPM, altough this is not true (bug #1). As we did write more than than TPM_BUFSIZE bytes but tpm_write reports that only TPM_BUFSIZE bytes have been written the vfs tries to write the remaining bytes (in this case 10 bytes) to the tpm device driver via tpm_write which then blocks at /* cannot perform a write until the read has cleared either via tpm_read or a user_read_timer timeout */ while (atomic_read(&chip->data_pending) != 0) msleep(TPM_TIMEOUT); for 60 seconds, since data_pending is -7 and nobody is able to read it (since tpm_read luckily checks if data_pending is greater than 0) (#bug 2). After that the remaining bytes are written to the TPM which are interpreted by the tpm as a normal command. (bug #3) So if the last bytes of the command stream happen to be a e.g. tpm_force_clear this gets accidentally sent to the TPM. This patch fixes all three bugs, by propagating the error code of tpm_write and returning -E2BIG if the input buffer is too big, since the response from the tpm for a truncated value is bogus anyway. Moreover it returns -EBUSY to userspace if there is a response ready to be read. Signed-off-by: Peter Huewe Signed-off-by: Kent Yoder diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c index 6724615..4caef33 100644 --- a/drivers/char/tpm/tpm.c +++ b/drivers/char/tpm/tpm.c @@ -1182,17 +1182,20 @@ ssize_t tpm_write(struct file *file, const char __user *buf, size_t size, loff_t *off) { struct tpm_chip *chip = file->private_data; - size_t in_size = size, out_size; + size_t in_size = size; + ssize_t out_size; /* cannot perform a write until the read has cleared - either via tpm_read or a user_read_timer timeout */ - while (atomic_read(&chip->data_pending) != 0) - msleep(TPM_TIMEOUT); - - mutex_lock(&chip->buffer_mutex); + either via tpm_read or a user_read_timer timeout. + This also prevents splitted buffered writes from blocking here. + */ + if (atomic_read(&chip->data_pending) != 0) + return -EBUSY; if (in_size > TPM_BUFSIZE) - in_size = TPM_BUFSIZE; + return -E2BIG; + + mutex_lock(&chip->buffer_mutex); if (copy_from_user (chip->data_buffer, (void __user *) buf, in_size)) { @@ -1202,6 +1205,10 @@ ssize_t tpm_write(struct file *file, const char __user *buf, /* atomic tpm command send and result receive */ out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE); + if (out_size < 0) { + mutex_unlock(&chip->buffer_mutex); + return out_size; + } atomic_set(&chip->data_pending, out_size); mutex_unlock(&chip->buffer_mutex); -- cgit v0.10.2