| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YtjqdJcGpulWsBHs@nanopsycho>
Date: Thu, 21 Jul 2022 07:56:04 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>
Subject: Re: [net-next PATCH 2/2] ice: support dry run of a flash update to
validate firmware file
Wed, Jul 20, 2022 at 08:34:33PM CEST, jacob.e.keller@...el.com wrote:
>Now that devlink core flash update can handle dry run requests, update
>the ice driver to allow validating a PLDM file in dry_run mode.
>
>First, add a new dry_run field to the pldmfw context structure. This
>indicates that the PLDM firmware file library should only validate the
>file and verify that it has a matching record. Update the pldmfw
>documentation to indicate this "dry run" mode.
>
>In the ice driver, let the stack know that we support the dry run
>attribute for flash update by setting the appropriate bit in the
>.supported_flash_update_params field.
>
>If the dry run is requested, notify the PLDM firmware library by setting
>the context bit appropriately. Don't cancel a pending update during
>a dry run.
>
>Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>---
> Documentation/driver-api/pldmfw/index.rst | 10 ++++++++++
> drivers/net/ethernet/intel/ice/ice_devlink.c | 3 ++-
> drivers/net/ethernet/intel/ice/ice_fw_update.c | 14 ++++++++++----
> include/linux/pldmfw.h | 5 +++++
> lib/pldmfw/pldmfw.c | 12 ++++++++++++
> 5 files changed, 39 insertions(+), 5 deletions(-)
>
>diff --git a/Documentation/driver-api/pldmfw/index.rst b/Documentation/driver-api/pldmfw/index.rst
>index ad2c33ece30f..454b3ed6576a 100644
>--- a/Documentation/driver-api/pldmfw/index.rst
>+++ b/Documentation/driver-api/pldmfw/index.rst
>@@ -51,6 +51,16 @@ unaligned access of multi-byte fields, and to properly convert from Little
> Endian to CPU host format. Additionally the records, descriptors, and
> components are stored in linked lists.
>
>+Validating a PLDM firmware file
>+===============================
>+
>+To simply validate a PLDM firmware file, and verify whether it applies to
>+the device, set the ``dry_run`` flag in the ``pldmfw`` context structure.
>+If this flag is set, the library will parse the file, validating its UUID
>+and checking if any record matches the device. Note that in a dry run, the
>+library will *not* issue any ops besides ``match_record``. It will not
>+attempt to send the component table or package data to the device firmware.
>+
> Performing a flash update
> =========================
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
>index 3337314a7b35..18214ea33e2d 100644
>--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>@@ -467,7 +467,8 @@ ice_devlink_reload_empr_finish(struct devlink *devlink,
> }
>
> static const struct devlink_ops ice_devlink_ops = {
>- .supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,
>+ .supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK |
>+ DEVLINK_SUPPORT_FLASH_UPDATE_DRY_RUN,
> .reload_actions = BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
> /* The ice driver currently does not support driver reinit */
> .reload_down = ice_devlink_reload_empr_start,
>diff --git a/drivers/net/ethernet/intel/ice/ice_fw_update.c b/drivers/net/ethernet/intel/ice/ice_fw_update.c
>index 3dc5662d62a6..63317ae88186 100644
>--- a/drivers/net/ethernet/intel/ice/ice_fw_update.c
>+++ b/drivers/net/ethernet/intel/ice/ice_fw_update.c
>@@ -1015,15 +1015,21 @@ int ice_devlink_flash_update(struct devlink *devlink,
> else
> priv.context.ops = &ice_fwu_ops_e810;
> priv.context.dev = dev;
>+ priv.context.dry_run = params->dry_run;
> priv.extack = extack;
> priv.pf = pf;
> priv.activate_flags = preservation;
>
>- devlink_flash_update_status_notify(devlink, "Preparing to flash", NULL, 0, 0);
>+ if (params->dry_run)
>+ devlink_flash_update_status_notify(devlink, "Validating flash binary", NULL, 0, 0);
You do validation of the binary instead of the actual flash. Why is it
called "dry-run" then? Perhaps "validate" would be more suitable?
>+ else
>+ devlink_flash_update_status_notify(devlink, "Preparing to flash", NULL, 0, 0);
>
>- err = ice_cancel_pending_update(pf, NULL, extack);
>- if (err)
>- return err;
>+ if (!params->dry_run) {
>+ err = ice_cancel_pending_update(pf, NULL, extack);
>+ if (err)
>+ return err;
>+ }
>
> err = ice_acquire_nvm(hw, ICE_RES_WRITE);
> if (err) {
>diff --git a/include/linux/pldmfw.h b/include/linux/pldmfw.h
>index 0fc831338226..d9add301582b 100644
>--- a/include/linux/pldmfw.h
>+++ b/include/linux/pldmfw.h
>@@ -124,10 +124,15 @@ struct pldmfw_ops;
> * should embed this in a private structure and use container_of to obtain
> * a pointer to their own data, used to implement the device specific
> * operations.
>+ *
>+ * @ops: function pointers used as callbacks from the PLDMFW library
>+ * @dev: pointer to the device being updated
>+ * @dry_run: if true, only validate the file, do not perform an update.
> */
> struct pldmfw {
> const struct pldmfw_ops *ops;
> struct device *dev;
>+ bool dry_run;
> };
>
> bool pldmfw_op_pci_match_record(struct pldmfw *context, struct pldmfw_record *record);
>diff --git a/lib/pldmfw/pldmfw.c b/lib/pldmfw/pldmfw.c
>index 6e77eb6d8e72..29a132a39876 100644
>--- a/lib/pldmfw/pldmfw.c
>+++ b/lib/pldmfw/pldmfw.c
>@@ -827,6 +827,10 @@ static int pldm_finalize_update(struct pldmfw_priv *data)
> * to the device firmware. Extract and write the flash data for each of the
> * components indicated in the firmware file.
> *
>+ * If the context->dry_run is set, this is a request for a dry run, i.e. to
>+ * only validate the PLDM firmware file. In this case, stop and exit after we
>+ * find a valid matching record.
>+ *
> * Returns: zero on success, or a negative error code on failure.
> */
> int pldmfw_flash_image(struct pldmfw *context, const struct firmware *fw)
>@@ -844,14 +848,22 @@ int pldmfw_flash_image(struct pldmfw *context, const struct firmware *fw)
> data->fw = fw;
> data->context = context;
>
>+ /* Parse the image and make sure it is a valid PLDM firmware binary */
> err = pldm_parse_image(data);
> if (err)
> goto out_release_data;
>
>+ /* Search for a record matching the device */
> err = pldm_find_matching_record(data);
> if (err)
> goto out_release_data;
>
>+ /* If this is a dry run, do not perform an update */
>+ if (context->dry_run)
>+ goto out_release_data;
>+
>+ /* Perform the device update */
>+
> err = pldm_send_package_data(data);
> if (err)
> goto out_release_data;
>--
>2.35.1.456.ga9c7032d4631
>
Powered by blists - more mailing lists