ACK/Cmnt: [SRU][F:linux-bluefield][PATCH v5 0/4] Updates to mlx-bootctl

[Stefan Bader]

On 07.07.21 14:53, Tim Gardner wrote:
> Acked-by: Tim Gardner <tim.gardner at canonical.com>
> 
> There is a 'for' loop in rsh_log_show_crash() that _might_ overflow 'buf'. It 
> depends on how the kernel implementation of snprintf() handles a length of 0. A 
> way around that is:
> 
> for (i = 0; i < len/2 && size > 0; i++) {
> 
> This patch series is fine as is. If you're concerned about that 'for' loop, then 
> you can send a follow-on patch that references the same bug number.

Not if this set has already been applied by then. Partial adjustments under the 
same bug reference have a high risk of going wrong and once committed it is also 
causing confusion. Either the whole set should be NACKed and and re-submitted or 
the loop issue should be handled in a separate thread.

-Stefan

> 
> rtg
> 
> On 7/7/21 2:16 AM, Shravan Kumar Ramani wrote:
>> v4 --> v5
>> Add more details to commit message and include missing "BugLink:" key.
>> Use one common bug report for all patches in the series.
>>
>> v3 --> v4
>> Redo split as follows:
>> 1. Fix exclusion issues around arm_smccc_smc
>> 2. Fix potential buffer overflow
>> 3. Support VPD info in EEPROM MFG
>> 4. Update license and version info
>>
>> 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.
>>
>> 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.
>>
>> This patch series contains the following 4 patches:
>> 1. Fix exclusion issues around arm_smccc_smc
>> Exclusion is being handled around arm_smccc_smc() only in the store functions. 
>> It should be implemented in the show functions also as this call might not be 
>> thread-safe.
>> Add mutex_lock/unlock around arm_smccc_smc calls in the show functions.
>>
>> 2. Fix potential buffer overflow
>> The sysfs store/show functions use sprintf without specifying a size which 
>> could lead to potential buffer overflow.
>> Replace sprintf with snprintf to avoid buffer overflow. Also, remove the 
>> redundant strlen usage since count is already available in the _store functions.
>>
>> 3. Support VPD info in EEPROM MFG
>> 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.
>> 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.
>>
>> 4. Update license and version info
>> Update license info to "Dual BSD/GPL".
>> Increment version to 1.4
>>
>> Shravan Kumar Ramani (4):
>>    UBUNTU: SAUCE: mlx-bootctl: Fix exclusion issues around arm_smccc_smc
>>    UBUNTU: SAUCE: mlx-bootctl: Fix potential buffer overflow
>>    UBUNTU: SAUCE: mlx-bootctl: Support VPD info in EEPROM MFG
>>    UBUNTU: SAUCE: mlx-bootctl: Update license and version info
>>
>>   drivers/platform/mellanox/mlx-bootctl.c | 373 ++++++++++++++++++------
>>   1 file changed, 290 insertions(+), 83 deletions(-)
>>
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210708/726ee917/attachment-0001.sig>