Re: [tpmdd-devel] Re: [PATCH 1/1] tpm: update tpm sysfs file ownership- updated version

From: Kylene Hall
Date: Wed Feb 09 2005 - 15:36:58 EST


On Wed, 9 Feb 2005, Greg KH wrote:
> On Wed, Feb 09, 2005 at 12:05:42PM -0600, Kylene Hall wrote:
> > @@ -539,9 +551,8 @@ void tpm_remove_hardware(struct device *
> > dev_set_drvdata(dev, NULL);
> > misc_deregister(&chip->vendor->miscdev);
> >
> > - device_remove_file(dev, &dev_attr_pubek);
> > - device_remove_file(dev, &dev_attr_pcrs);
> > - device_remove_file(dev, &dev_attr_caps);
> > + for ( i = 0; i < TPM_ATTRS; i++ )
> > + device_remove_file(dev, &chip->attr[i]);
> >
> > dev_mask[chip->dev_num / 32] &= !(1 << (chip->dev_num % 32));
> >
>
> This code works?
>
> > @@ -608,6 +619,11 @@ int tpm_register_hardware(struct device
> > struct tpm_chip *chip;
> > int i, j;
> >
> > + DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
> > + DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
> > + DEVICE_ATTR(caps, S_IRUGO, show_caps, NULL);
> > + DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, store_cancel);
> > +
> > /* Driver specific per-device data */
> > chip = kmalloc(sizeof(*chip), GFP_KERNEL);
> > if (chip == NULL)
>
> You do realize you just created those attributes on the stack? And then
> you try to remove them from within a different scope above?
>
I didn't realize their was a string in the structure that wouldn't get
copied when I made the copy to the kmalloc'd struct. Here is an alternate
implementation that pushes the initialization into the respective specific
drivers via a MACRO which has the added benefit of getting the owner field
right from the start.

> thanks,
>
> greg k-h

Signed-off-by: Kylene Hall <kjhall@xxxxxxxxxx>
---

diff -uprN linux-2.6.10/drivers/char/tpm/tpm_atmel.c linux-2.6.10-tpm/drivers/char/tpm/tpm_atmel.c
--- linux-2.6.10/drivers/char/tpm/tpm_atmel.c 2005-02-04 15:03:03.000000000 -0600
+++ linux-2.6.10-tpm/drivers/char/tpm/tpm_atmel.c 2005-02-09 14:12:30.711621784 -0600
@@ -131,6 +131,7 @@ static struct tpm_vendor_specific tpm_at
.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
.req_complete_val = ATML_STATUS_DATA_AVAIL,
.base = TPM_ATML_BASE,
+ .attr = TPM_DEVICE_ATTRS,
.miscdev.fops = &atmel_ops,
};

diff -uprN linux-2.6.10/drivers/char/tpm/tpm.c linux-2.6.10-tpm/drivers/char/tpm/tpm.c
--- linux-2.6.10/drivers/char/tpm/tpm.c 2005-02-04 15:03:03.000000000 -0600
+++ linux-2.6.10-tpm/drivers/char/tpm/tpm.c 2005-02-09 14:12:30.695624216 -0600
@@ -213,7 +213,7 @@ static u8 pcrread[] = {
0, 0, 0, 0 /* PCR index */
};

-static ssize_t show_pcrs(struct device *dev, char *buf)
+ssize_t show_pcrs(struct device *dev, char *buf)
{
u8 data[READ_PCR_RESULT_SIZE];
ssize_t len;
@@ -245,8 +245,7 @@ static ssize_t show_pcrs(struct device *
}
return str - buf;
}
-
-static DEVICE_ATTR(pcrs, S_IRUGO, show_pcrs, NULL);
+EXPORT_SYMBOL_GPL(show_pcrs);

#define READ_PUBEK_RESULT_SIZE 314
static u8 readpubek[] = {
@@ -255,7 +254,7 @@ static u8 readpubek[] = {
0, 0, 0, 124, /* TPM_ORD_ReadPubek */
};

-static ssize_t show_pubek(struct device *dev, char *buf)
+ssize_t show_pubek(struct device *dev, char *buf)
{
u8 data[READ_PUBEK_RESULT_SIZE];
ssize_t len;
@@ -308,7 +307,7 @@ static ssize_t show_pubek(struct device
return str - buf;
}

-static DEVICE_ATTR(pubek, S_IRUGO, show_pubek, NULL);
+EXPORT_SYMBOL_GPL(show_pubek);

#define CAP_VER_RESULT_SIZE 18
static u8 cap_version[] = {
@@ -329,7 +328,7 @@ static u8 cap_manufacturer[] = {
0, 0, 1, 3
};

-static ssize_t show_caps(struct device *dev, char *buf)
+ssize_t show_caps(struct device *dev, char *buf)
{
u8 data[READ_PUBEK_RESULT_SIZE];
ssize_t len;
@@ -362,7 +361,26 @@ static ssize_t show_caps(struct device *
return str - buf;
}

-static DEVICE_ATTR(caps, S_IRUGO, show_caps, NULL);
+EXPORT_SYMBOL_GPL(show_caps);
+
+ssize_t store_cancel(struct device *dev, const char *buf,
+ size_t count)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ if (chip == NULL)
+ return 0;
+
+ chip->vendor->cancel(chip);
+
+ down(&chip->timer_manipulation_mutex);
+ if (timer_pending(&chip->device_timer))
+ mod_timer(&chip->device_timer, jiffies);
+ up(&chip->timer_manipulation_mutex);
+
+ return count;
+}
+
+EXPORT_SYMBOL_GPL(store_cancel);

/*
* Device file system interface to the TPM
@@ -524,6 +542,7 @@ EXPORT_SYMBOL_GPL(tpm_read);
void tpm_remove_hardware(struct device *dev)
{
struct tpm_chip *chip = dev_get_drvdata(dev);
+ int i;

if (chip == NULL) {
dev_err(dev, "No device data found\n");
@@ -539,9 +558,8 @@ void tpm_remove_hardware(struct device *
dev_set_drvdata(dev, NULL);
misc_deregister(&chip->vendor->miscdev);

- device_remove_file(dev, &dev_attr_pubek);
- device_remove_file(dev, &dev_attr_pcrs);
- device_remove_file(dev, &dev_attr_caps);
+ for ( i = 0; i < TPM_NUM_ATTR; i++ )
+ device_remove_file(dev, &chip->vendor->attr[i]);

dev_mask[chip->dev_num / 32] &= !(1 << (chip->dev_num % 32));

@@ -663,10 +681,9 @@ dev_num_search_complete:

list_add(&chip->list, &tpm_chip_list);

- device_create_file(dev, &dev_attr_pubek);
- device_create_file(dev, &dev_attr_pcrs);
- device_create_file(dev, &dev_attr_caps);
-
+ for ( i = 0; i < TPM_NUM_ATTR; i++ )
+ device_create_file(dev, &chip->vendor->attr[i]);
+
return 0;
}

diff -uprN linux-2.6.10/drivers/char/tpm/tpm.h linux-2.6.10-tpm/drivers/char/tpm/tpm.h
--- linux-2.6.10/drivers/char/tpm/tpm.h 2005-02-04 15:03:03.000000000 -0600
+++ linux-2.6.10-tpm/drivers/char/tpm/tpm.h 2005-02-09 14:12:30.702623152 -0600
@@ -25,11 +25,23 @@
#include <linux/miscdevice.h>

#define TPM_TIMEOUT msecs_to_jiffies(5)
+#define TPM_NUM_ATTR 4

/* TPM addresses */
#define TPM_ADDR 0x4E
#define TPM_DATA 0x4F

+extern ssize_t show_pubek(struct device *, char *);
+extern ssize_t show_pcrs(struct device *, char *);
+extern ssize_t show_caps(struct device *, char *);
+extern ssize_t store_cancel(struct device *, const char *, size_t);
+
+#define TPM_DEVICE_ATTRS { \
+ __ATTR(pubek, S_IRUGO, show_pubek, NULL), \
+ __ATTR(pcrs, S_IRUGO, show_pcrs, NULL), \
+ __ATTR(caps, S_IRUGO, show_caps, NULL), \
+ __ATTR(cancel, S_IWUSR | S_IWGRP, NULL, store_cancel) }
+
struct tpm_chip;

struct tpm_vendor_specific {
@@ -42,6 +54,7 @@ struct tpm_vendor_specific {
void (*cancel) (struct tpm_chip *);
u8 (*status) (struct tpm_chip *);
struct miscdevice miscdev;
+ struct device_attribute attr[TPM_NUM_ATTR];
};

struct tpm_chip {
diff -uprN linux-2.6.10/drivers/char/tpm/tpm_nsc.c linux-2.6.10-tpm/drivers/char/tpm/tpm_nsc.c
--- linux-2.6.10/drivers/char/tpm/tpm_nsc.c 2005-02-04 15:03:03.000000000 -0600
+++ linux-2.6.10-tpm/drivers/char/tpm/tpm_nsc.c 2005-02-09 14:12:30.728619200 -0600
@@ -241,6 +241,7 @@ static struct tpm_vendor_specific tpm_ns
.req_complete_mask = NSC_STATUS_OBF,
.req_complete_val = NSC_STATUS_OBF,
.base = TPM_NSC_BASE,
+ .attr = TPM_DEVICE_ATTRS,
.miscdev.fops = &nsc_ops,
};

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/