[SRU,F:linux-bluefield,v3,0/3] Updates to mlx-bootctl

v2 --> v3
Add mutex lock/unlock for SMC calls in show functions, similar to store.
Use PAGE_SIZE macro as buffer size for snprintf calls in DRIVER_ATTR_RW functions.
Use snprintf instead of sprintf in post_reset_wdog_show.
In secure_boot_fuse_state_show, the string comes from the function itself unlike the rest. So it is protected against buffer overflow.

v1 --> v2
Split single patch into 3 separate patches based on functionality as suggested.

1. Support VPD info in EEPROM MFG
BugLink: https://bugs.launchpad.net/bugs/1931843
SRU Justification:

[Impact]
The EEPROM MFG partition on BlueField-2 has been updated to include the VPD information for each card. In order to access these newly added fields, the mlx-bootctl driver needs to be updated to provide an access mechanism.

[Fix]
Add support for VPD fields in the EEPROM MFG and provide access to these via sysfs entries. The newly added sysfs entries are: sku (SKU ID), modl (Model Number), sn (Serial Number) and uuid (UUID). And the previously added opn_str sysfs has been renamed to opn.

[Test Case]
Though the driver provides read and write access through sysfs, the contents of the MFG partition are written during Manufacturing and then locked in order to protect the info. Writing to this partition will therefore require resetting the MFG info from the UEFI Device Manager, which will unlock the partition and allow for it to be reprogrammed.
Reading the sysfs entries will print the contents of each field. It could also be empty if the field was not programmed earlier.

[Regression Potential]
Can be considered minimum, since the new fields have been added without interfering with the existing fields which might already be present in the EEPROM.

2. Fix potential buffer overflow
BugLink: https://bugs.launchpad.net/bugs/1931981
SRU Justification:

[Impact]
The sysfs store/show functions use sprintf without specifying a size which could lead to potential buffer overflow.

[Fix]
Replace sprintf with snprintf to avoid buffer overflow. Also, remove the redundant strlen usage since count is already available in the _store functions.

[Test Plan]
Read/write access to the EEPROM MFG fields can be tested via the sysfs entries that are exposed by the driver. Please note that the MFG partition is locked in order to protect the data and this could block all writes to it. In order to enable writes to the EEPROM, the MFG Info needs to be reset via the UEFI Device Manager.

[Regression Potential]
Can be considered minimum.

3. Update license and version info
BugLink: https://bugs.launchpad.net/bugs/1931984
SRU Justification:

[Impact]
License info needs to be updated since the current info is no longer accurate.
Driver version needs to be incremented since new features have been added.

[Fix]
Update license info to "Dual BSD/GPL".
Increment version to 1.4

[Test Plan]
Verify version change

[Regression Potential]
None

Shravan Kumar Ramani (3):
  UBUNTU: SAUCE: mlx-bootctl: Support VPD info in EEPROM MFG
  UBUNTU: SAUCE: mlx-bootctl: Fix potential buffer overflow
  UBUNTU: SAUCE: mlx-bootctl: Update license and version info

 drivers/platform/mellanox/mlx-bootctl.c | 373 ++++++++++++++++++------
 1 file changed, 289 insertions(+), 84 deletions(-)

You are still mixing multiple changes in patch 2, i.e., fixing sprintf() 
buffer overflows while at the same time implementing exclusion in the 
show functions. I would implement this patch set like this:

1: Fix existing exclusion issues around arm_smccc_smc()
2: Fix existing sprintf buffer overflow issues.
3: Implement VPD support keeping in mind the issues you've just fixed in 
patches 1 & 2.
4: Change the license and version.

Also, this macro is disingenuous:

#define MLNX_MFG_VAL_WORD_CNT(type)  (MLNX_MFG_##type##_VAL_LEN / 8)

A WORD in the world I come from is 2 bytes. I think it would be better 
defined as:

#define MLNX_MFG_VAL_QWORD_CNT(type)  (MLNX_MFG_##type##_VAL_LEN / 
sizeof(u64))

Its also more consistent with the for loops used to read arm_smcc_smc() 
since you're using sizeof(u64) as the read length.

rtg

On 6/16/21 12:41 AM, Shravan Kumar Ramani wrote:
> v2 --> v3
> Add mutex lock/unlock for SMC calls in show functions, similar to store.
> Use PAGE_SIZE macro as buffer size for snprintf calls in DRIVER_ATTR_RW functions.
> Use snprintf instead of sprintf in post_reset_wdog_show.
> In secure_boot_fuse_state_show, the string comes from the function itself unlike the rest. So it is protected against buffer overflow.
> 
> v1 --> v2
> Split single patch into 3 separate patches based on functionality as suggested.
> 
> 1. Support VPD info in EEPROM MFG
> BugLink: https://bugs.launchpad.net/bugs/1931843
> SRU Justification:
> 
> [Impact]
> The EEPROM MFG partition on BlueField-2 has been updated to include the VPD information for each card. In order to access these newly added fields, the mlx-bootctl driver needs to be updated to provide an access mechanism.
> 
> [Fix]
> Add support for VPD fields in the EEPROM MFG and provide access to these via sysfs entries. The newly added sysfs entries are: sku (SKU ID), modl (Model Number), sn (Serial Number) and uuid (UUID). And the previously added opn_str sysfs has been renamed to opn.
> 
> [Test Case]
> Though the driver provides read and write access through sysfs, the contents of the MFG partition are written during Manufacturing and then locked in order to protect the info. Writing to this partition will therefore require resetting the MFG info from the UEFI Device Manager, which will unlock the partition and allow for it to be reprogrammed.
> Reading the sysfs entries will print the contents of each field. It could also be empty if the field was not programmed earlier.
> 
> [Regression Potential]
> Can be considered minimum, since the new fields have been added without interfering with the existing fields which might already be present in the EEPROM.
> 
> 2. Fix potential buffer overflow
> BugLink: https://bugs.launchpad.net/bugs/1931981
> SRU Justification:
> 
> [Impact]
> The sysfs store/show functions use sprintf without specifying a size which could lead to potential buffer overflow.
> 
> [Fix]
> Replace sprintf with snprintf to avoid buffer overflow. Also, remove the redundant strlen usage since count is already available in the _store functions.
> 
> [Test Plan]
> Read/write access to the EEPROM MFG fields can be tested via the sysfs entries that are exposed by the driver. Please note that the MFG partition is locked in order to protect the data and this could block all writes to it. In order to enable writes to the EEPROM, the MFG Info needs to be reset via the UEFI Device Manager.
> 
> [Regression Potential]
> Can be considered minimum.
> 
> 3. Update license and version info
> BugLink: https://bugs.launchpad.net/bugs/1931984
> SRU Justification:
> 
> [Impact]
> License info needs to be updated since the current info is no longer accurate.
> Driver version needs to be incremented since new features have been added.
> 
> [Fix]
> Update license info to "Dual BSD/GPL".
> Increment version to 1.4
> 
> [Test Plan]
> Verify version change
> 
> [Regression Potential]
> None
> 
> Shravan Kumar Ramani (3):
>    UBUNTU: SAUCE: mlx-bootctl: Support VPD info in EEPROM MFG
>    UBUNTU: SAUCE: mlx-bootctl: Fix potential buffer overflow
>    UBUNTU: SAUCE: mlx-bootctl: Update license and version info
> 
>   drivers/platform/mellanox/mlx-bootctl.c | 373 ++++++++++++++++++------
>   1 file changed, 289 insertions(+), 84 deletions(-)
>