* Kylene Hall ([EMAIL PROTECTED]) wrote:
> 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)

This is too generic a name for global namespace.

>  {
>       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)

same here

>  {
>       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)

and here.

>  {
>       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)

and here

> +{
> +     struct tpm_chip *chip = dev_get_drvdata(dev);
> +     if (chip == NULL)
> +             return 0;
> +

Do you want any extra protection besides mode bits (S_IWUSR | S_IWGRP)?
How privileged should this operation be?

> +     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) }

This doesn't look like the right way to go.  

> +
>  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];

So every device will have the same attrs?  If so, make that whole struct
exported (not the individual show/store methods) and reference that in
each driver.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net
-
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/

Reply via email to