Flipping firmware write-protection bits from within the kernel (was Re: [PATCH RFC platform-next 8/8] Documentation/ABI: Add new line card attributes for mlxreg-io sysfs interfaces)

[Kees Cook]

On Thu, Mar 04, 2021 at 11:48:10AM +0100, Hans de Goede wrote:
> Hi Kees, et al.,
> 
> Kees, you were the first person who came to mind to ask about this, feel free to
> point me to someone else.
> 
> While reviewing a patch-set for hw-enablement of some upcoming Melanox platforms,
> the addition of some sysfs files which flip write-protect bits for various firmwares
> found on the platform on/off stood out to me.

Eeek :(

> As I mention in me reply to the below patch adding the docs for this at a minimum
> this must (IMHO) tie into the new lockdown framework and disallow enabling
> fw-updates this way depending on the lockdown mode.

I would agree: lockdown (integrity mode) should, IMO, block these kinds
of firmware writes if there isn't some kind of cryptographic attestation
happening.

(Probably there are many of these holes already in the kernel, but we
should avoid adding more.)

> 
> Are there any other security checks which the code should/could do here ?
> 
> Maybe tie into the audit code somehow ?

I'll leave that to the audit folks to answer. :)

-Kees

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> On 3/4/21 11:39 AM, Hans de Goede wrote:
> > Hi,
> > 
> > On 2/3/21 6:36 PM, Vadim Pasternak wrote:
> >> Add documentation for the new attributes for line cards:
> >> - CPLDs versioning.
> >> - Write protection control for 'nvram' devices.
> >> - Line card reset reasons.
> >> - Enabling burning of FPGA and CPLDs.
> >> - Enabling burning of FPGA and gearbox SPI flashes,
> >> - Enabling power of whole line card.
> >> - Enabling power of QSFP ports equipped on line card.
> >> - The maximum powered required for line card feeding.
> >> - Line card configuration Id.
> >>
> >> Signed-off-by: Vadim Pasternak <vadimp at nvidia.com>
> >> ---
> >>  Documentation/ABI/stable/sysfs-driver-mlxreg-io | 92 +++++++++++++++++++++++++
> >>  1 file changed, 92 insertions(+)
> >>
> >> diff --git a/Documentation/ABI/stable/sysfs-driver-mlxreg-io b/Documentation/ABI/stable/sysfs-driver-mlxreg-io
> >> index 1d1a8ee59534..a22e9d6c0904 100644
> >> --- a/Documentation/ABI/stable/sysfs-driver-mlxreg-io
> >> +++ b/Documentation/ABI/stable/sysfs-driver-mlxreg-io
> >> @@ -326,3 +326,95 @@ Description:	This file unlocks system after hardware or firmware thermal
> >>  		locking.
> >>  
> >>  		The file is read/write.
> >> +
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/cpld1_pn
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/cpld1_version
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/cpld1_version_min
> >> +Date:		March 2021
> >> +KernelVersion:	5.12
> >> +Contact:	Vadim Pasternak <vadimp at nvidia.com>
> >> +Description:	These files show with which CPLD major and minor versions
> >> +		and part number has been burned CPLD device on line card.
> >> +
> >> +		The files are read only.
> >> +
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/fpga1_pn
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/fpga1_version
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/fpga1_version_min
> >> +Date:		March 2021
> >> +KernelVersion:	5.12
> >> +Contact:	Vadim Pasternak <vadimp at nvidia.com>
> >> +Description:	These files show with which FPGA major and minor versions
> >> +		and part number has been burned FPGA device on line card.
> >> +
> >> +		The files are read only.
> >> +
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/ini_wp
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/vpd_wp
> >> +Date:		March 2021
> >> +KernelVersion:	5.12
> >> +Contact:	Vadim Pasternak <vadimp at nvidia.com>
> >> +Description:	These files allow to overwrite line card VPD and firmware blob
> >> +		hardware write protection mode. When attribute is set 1 - write
> >> +		protection is disabled, when 0 - enabled. By default both are
> >> +		write protected.
> >> +
> >> +		The files are read/write.
> > 
> > This seems to have some serious security implications. IMHO this should be tie into the
> > kernel's new(ish) lockdown system, so that if the system is in locked-down mode writing
> > these will not be allowed.
> > 
> >> +
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/reset_aux_pwr_or_ref
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/reset_dc_dc_pwr_fail
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/reset_fpga_not_done
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/reset_from_chassis
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/reset_line_card
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/reset_pwr_off_from_chassis
> >> +Date:		March 2021
> >> +KernelVersion:	5.12
> >> +Contact:	Vadim Pasternak <vadimp at nvidia.com>
> >> +Description:	These files show the line reset cause, as following: power
> >> +		auxiliary outage or power refresh, DC-to-DC power failure, FPGA reset
> >> +		failed, line card reset failed, power off from chassis.
> >> +		Value 1 in file means this is reset cause, 0 - otherwise. Only one of
> >> +		the above causes could be 1 at the same time, representing only last
> >> +		reset cause.
> >> +
> >> +		The files are read only.
> >> +
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/cpld_upgrade_en
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/fpga_upgrade_en
> >> +Date:		March 2021
> >> +KernelVersion:	5.12
> >> +Contact:	Vadim Pasternak <vadimp at nvidia.com>
> >> +Description:	These files allow CPLD and FPGA burning. Value 1 in file means burning
> >> +		is enabled, 0 - otherwise.
> >> +
> >> +		The files are read/write.
> > 
> > Same remark as above.
> > 
> >> +
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/qsfp_pwr_en
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/pwr_en
> >> +Date:		March 2021
> >> +KernelVersion:	5.12
> >> +Contact:	Vadim Pasternak <vadimp at nvidia.com>
> >> +Description:	These files allow to power on/off all QSFP ports and whole line card.
> >> +		The attributes are set 1 for power on, 0 - for power off.
> >> +
> >> +		The files are read/write.
> >> +
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/agb_spi_burn_en
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/fpga_spi_burn_en
> >> +Date:		March 2021
> >> +KernelVersion:	5.12
> >> +Contact:	Vadim Pasternak <vadimp at nvidia.com>
> >> +Description:	These files allow gearboxes and FPGA SPI flash burning.
> >> +		The attributes are set 1 to enable burning, 0 - to disable.
> >> +
> >> +		The file is read/write.
> > 
> > Same remark as above.
> > 
> >> +
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/max_power
> >> +What:		/sys/devices/platform/mlxplat/i2c_mlxcpld.*/i2c-*/i2c-*/i2c-*/*-0032/mlxreg-io.*/hwmon/hwmon*/config
> >> +Date:		March 2021
> >> +KernelVersion:	5.12
> >> +Contact:	Vadim Pasternak <vadimp at nvidia.com>
> >> +Description:	These files provide the maximum powered required for line card
> >> +		feeding and line card configuration Id.
> >> +
> >> +		The files are read only.
> >>
> > 
> > Regards,
> > 
> > Hans
> > 
> 

-- 
Kees Cook