linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 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)
       [not found]   ` <009a1a80-62ca-35ce-5f02-b43b25e5ebd1@redhat.com>
@ 2021-03-04 10:48     ` Hans de Goede
  2021-03-04 20:06       ` Kees Cook
  0 siblings, 1 reply; 2+ messages in thread
From: Hans de Goede @ 2021-03-04 10:48 UTC (permalink / raw)
  To: Vadim Pasternak, Kees Cook
  Cc: platform-driver-x86, linux-security-module, Andy Shevchenko

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.

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.

Are there any other security checks which the code should/could do here ?

Maybe tie into the audit code somehow ?

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@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@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@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@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@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@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@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@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@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
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: 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)
  2021-03-04 10:48     ` 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) Hans de Goede
@ 2021-03-04 20:06       ` Kees Cook
  0 siblings, 0 replies; 2+ messages in thread
From: Kees Cook @ 2021-03-04 20:06 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Vadim Pasternak, platform-driver-x86, linux-security-module,
	Andy Shevchenko, linux-hardening, Matthew Garrett

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@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@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@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@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@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@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@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@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@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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-03-04 20:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210203173622.5845-1-vadimp@nvidia.com>
     [not found] ` <20210203173622.5845-9-vadimp@nvidia.com>
     [not found]   ` <009a1a80-62ca-35ce-5f02-b43b25e5ebd1@redhat.com>
2021-03-04 10:48     ` 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) Hans de Goede
2021-03-04 20:06       ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).