tpm: use test_bit() to check TPM2 flag in eventlog and sysfs code
diff mbox

Message ID 1479715431-11983-1-git-send-email-nayna@linux.vnet.ibm.com
State New
Headers show

Commit Message

Nayna Jain Nov. 21, 2016, 8:03 a.m. UTC
There is change done to introduce atomic bitops to set and test
chip->flags.
This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device()
to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-sysfs.c    | 2 +-
 drivers/char/tpm/tpm_eventlog.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Jarkko Sakkinen Nov. 21, 2016, 7:55 p.m. UTC | #1
On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote:
> There is change done to introduce atomic bitops to set and test
> chip->flags.
> This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device()
> to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag.
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

I'm bit lost of the purpose of this patch.

/Jarkko

> ---
>  drivers/char/tpm/tpm-sysfs.c    | 2 +-
>  drivers/char/tpm/tpm_eventlog.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 82298e51..9a37c26 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -284,7 +284,7 @@ static const struct attribute_group tpm_dev_group = {
>  
>  void tpm_sysfs_add_device(struct tpm_chip *chip)
>  {
> -	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +	if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
>  		return;
>  
>  	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index ebec4ac..dede2ec 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -391,7 +391,7 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>  	unsigned int cnt;
>  	int rc = 0;
>  
> -	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +	if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
>  		return 0;
>  
>  	rc = tpm_read_log(chip);
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nayna Jain Nov. 22, 2016, 9:02 a.m. UTC | #2
On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote:
> On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote:
>> There is change done to introduce atomic bitops to set and test
>> chip->flags.
>> This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device()
>> to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag.
>>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>
> I'm bit lost of the purpose of this patch.

I was using tabrm branch which has changes related to using bitops for 
chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() 
with the existing way of checking. Replacing existing one with 
test_bit() check makes it work. Same in case of tpm_sysfs_add_device().

Thanks & Regards,
   - Nayna

>
> /Jarkko
>
>> ---
>>   drivers/char/tpm/tpm-sysfs.c    | 2 +-
>>   drivers/char/tpm/tpm_eventlog.c | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>> index 82298e51..9a37c26 100644
>> --- a/drivers/char/tpm/tpm-sysfs.c
>> +++ b/drivers/char/tpm/tpm-sysfs.c
>> @@ -284,7 +284,7 @@ static const struct attribute_group tpm_dev_group = {
>>
>>   void tpm_sysfs_add_device(struct tpm_chip *chip)
>>   {
>> -	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> +	if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
>>   		return;
>>
>>   	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
>> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
>> index ebec4ac..dede2ec 100644
>> --- a/drivers/char/tpm/tpm_eventlog.c
>> +++ b/drivers/char/tpm/tpm_eventlog.c
>> @@ -391,7 +391,7 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
>>   	unsigned int cnt;
>>   	int rc = 0;
>>
>> -	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> +	if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
>>   		return 0;
>>
>>   	rc = tpm_read_log(chip);
>> --
>> 2.5.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Sakkinen Nov. 22, 2016, 10:59 a.m. UTC | #3
On Tue, Nov 22, 2016 at 02:32:00PM +0530, Nayna wrote:
> 
> 
> On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote:
> > On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote:
> > > There is change done to introduce atomic bitops to set and test
> > > chip->flags.
> > > This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device()
> > > to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag.
> > > 
> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > 
> > I'm bit lost of the purpose of this patch.
> 
> I was using tabrm branch which has changes related to using bitops for
> chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() with
> the existing way of checking. Replacing existing one with test_bit() check
> makes it work. Same in case of tpm_sysfs_add_device().

Why didn't you just response to the thread with a review comment
especially as the patch is not applied to the master branch?

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nayna Jain Nov. 22, 2016, 11:06 a.m. UTC | #4
On 11/22/2016 04:29 PM, Jarkko Sakkinen wrote:
> On Tue, Nov 22, 2016 at 02:32:00PM +0530, Nayna wrote:
>>
>>
>> On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote:
>>> On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote:
>>>> There is change done to introduce atomic bitops to set and test
>>>> chip->flags.
>>>> This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device()
>>>> to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag.
>>>>
>>>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>>>
>>> I'm bit lost of the purpose of this patch.
>>
>> I was using tabrm branch which has changes related to using bitops for
>> chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() with
>> the existing way of checking. Replacing existing one with test_bit() check
>> makes it work. Same in case of tpm_sysfs_add_device().
>
> Why didn't you just response to the thread with a review comment
> especially as the patch is not applied to the master branch?

Oh!! Ok.. Sorry, I should have done that.
Will take care next time.

So basically it needs to be fixed as part of that patch.

Thanks & Regards,
    - Nayna

>
> /Jarkko
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Sakkinen Nov. 22, 2016, 11:28 a.m. UTC | #5
On Tue, Nov 22, 2016 at 12:59:59PM +0200, Jarkko Sakkinen wrote:
> On Tue, Nov 22, 2016 at 02:32:00PM +0530, Nayna wrote:
> > 
> > 
> > On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote:
> > > On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote:
> > > > There is change done to introduce atomic bitops to set and test
> > > > chip->flags.
> > > > This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device()
> > > > to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag.
> > > > 
> > > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > > 
> > > I'm bit lost of the purpose of this patch.
> > 
> > I was using tabrm branch which has changes related to using bitops for
> > chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() with
> > the existing way of checking. Replacing existing one with test_bit() check
> > makes it work. Same in case of tpm_sysfs_add_device().
> 
> Why didn't you just response to the thread with a review comment
> especially as the patch is not applied to the master branch?

Anyway I updated the corresponding patches. Thanks for reporting these
issues!

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Sakkinen Nov. 22, 2016, 2:31 p.m. UTC | #6
On Tue, Nov 22, 2016 at 04:36:56PM +0530, Nayna wrote:
> 
> 
> On 11/22/2016 04:29 PM, Jarkko Sakkinen wrote:
> > On Tue, Nov 22, 2016 at 02:32:00PM +0530, Nayna wrote:
> > > 
> > > 
> > > On 11/22/2016 01:25 AM, Jarkko Sakkinen wrote:
> > > > On Mon, Nov 21, 2016 at 03:03:51AM -0500, Nayna Jain wrote:
> > > > > There is change done to introduce atomic bitops to set and test
> > > > > chip->flags.
> > > > > This patch fixes tpm_bios_log_setup() and tpm_sysfs_add_device()
> > > > > to use test_bit() to check for TPM_CHIP_FLAG_TPM2 flag.
> > > > > 
> > > > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > > > 
> > > > I'm bit lost of the purpose of this patch.
> > > 
> > > I was using tabrm branch which has changes related to using bitops for
> > > chip->flags, but it was failing for TPM2 check in tpm_bios_log_setup() with
> > > the existing way of checking. Replacing existing one with test_bit() check
> > > makes it work. Same in case of tpm_sysfs_add_device().
> > 
> > Why didn't you just response to the thread with a review comment
> > especially as the patch is not applied to the master branch?
> 
> Oh!! Ok.. Sorry, I should have done that.
> Will take care next time.
> 
> So basically it needs to be fixed as part of that patch.

NP. Thanks for reporting the issue!

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 82298e51..9a37c26 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -284,7 +284,7 @@  static const struct attribute_group tpm_dev_group = {
 
 void tpm_sysfs_add_device(struct tpm_chip *chip)
 {
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+	if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
 		return;
 
 	/* The sysfs routines rely on an implicit tpm_try_get_ops, device_del
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index ebec4ac..dede2ec 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -391,7 +391,7 @@  int tpm_bios_log_setup(struct tpm_chip *chip)
 	unsigned int cnt;
 	int rc = 0;
 
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+	if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
 		return 0;
 
 	rc = tpm_read_log(chip);