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

[Shravan Ramani]

Noted, I will fix the loop issue in a separate thread.

Thanks,
Shravan

> -----Original Message-----
> From: Stefan Bader <stefan.bader at canonical.com>
> Sent: Thursday, July 8, 2021 12:32 PM
> To: Tim Gardner <tim.gardner at canonical.com>; Shravan Ramani
> <shravankr at nvidia.com>; kernel-team at lists.ubuntu.com
> Subject: Re: ACK/Cmnt: [SRU][F:linux-bluefield][PATCH v5 0/4] Updates to
> mlx-bootctl
> 
> 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(-)
> >>
> >
>