| 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
| ||
|
Date: Thu, 21 Jul 2022 07:59:30 +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: [iproute2-next PATCH 3/3] devlink: add dry run attribute support
to devlink flash
Wed, Jul 20, 2022 at 08:34:49PM CEST, jacob.e.keller@...el.com wrote:
>Recent versions of the kernel support the DEVLINK_ATTR_DRY_RUN attribute
>which allows requesting a dry run of a command. A dry run is simply
>a request to validate that a command would work, without performing any
>destructive changes.
>
>The attribute is supported by the devlink flash update as a way to
>validate an update, including potentially the binary image, without
>modifying the device.
>
>Add a "dry_run" option to the command line parsing which will enable
>this attribute when requested.
>
>To avoid potential issues, only allow the attribute to be added to
>commands when the kernel recognizes it. This is important because some
>commands do not perform strict validation. If we were to add the
>attribute without this check, an old kernel may silently accept the
>command and perform an update even when dry_run was requested.
>
>Before adding the attribute, check the maximum attribute from the
>CTRL_CMD_GETFAMILY and make sure that the kernel recognizes the
>DEVLINK_ATTR_DRY_RUN attribute.
>
>Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>---
> devlink/devlink.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 43 insertions(+), 2 deletions(-)
>
>diff --git a/devlink/devlink.c b/devlink/devlink.c
>index ddf430bbb02a..5649360b1417 100644
>--- a/devlink/devlink.c
>+++ b/devlink/devlink.c
>@@ -294,6 +294,7 @@ static void ifname_map_free(struct ifname_map *ifname_map)
> #define DL_OPT_PORT_FN_RATE_TX_MAX BIT(49)
> #define DL_OPT_PORT_FN_RATE_NODE_NAME BIT(50)
> #define DL_OPT_PORT_FN_RATE_PARENT BIT(51)
>+#define DL_OPT_DRY_RUN BIT(52)
>
> struct dl_opts {
> uint64_t present; /* flags of present items */
>@@ -368,6 +369,8 @@ struct dl {
> bool verbose;
> bool stats;
> bool hex;
>+ bool max_attr_valid;
>+ uint32_t max_attr;
> struct {
> bool present;
> char *bus_name;
>@@ -693,6 +696,7 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
> [DEVLINK_ATTR_TRAP_POLICER_ID] = MNL_TYPE_U32,
> [DEVLINK_ATTR_TRAP_POLICER_RATE] = MNL_TYPE_U64,
> [DEVLINK_ATTR_TRAP_POLICER_BURST] = MNL_TYPE_U64,
>+ [DEVLINK_ATTR_DRY_RUN] = MNL_TYPE_FLAG,
> };
>
> static const enum mnl_attr_data_type
>@@ -1512,6 +1516,30 @@ static int dl_args_finding_required_validate(uint64_t o_required,
> return 0;
> }
>
>+static void dl_get_max_attr(struct dl *dl)
>+{
>+ if (!dl->max_attr_valid) {
>+ uint32_t max_attr;
>+ int err;
>+
>+ err = mnlg_socket_get_max_attr(&dl->nlg, &max_attr);
>+ if (err) {
>+ pr_err("Unable to determine maximum supported devlink attribute\n");
>+ return;
>+ }
>+
>+ dl->max_attr = max_attr;
>+ dl->max_attr_valid = true;
>+ }
>+}
>+
>+static bool dl_kernel_supports_dry_run(struct dl *dl)
>+{
>+ dl_get_max_attr(dl);
>+
>+ return (dl->max_attr_valid && dl->max_attr >= DEVLINK_ATTR_DRY_RUN);
This looks like it would be handy for other attrs too. Could you make
this a generic helper accepting attr type as function argument?
>+}
>+
> static int dl_argv_parse(struct dl *dl, uint64_t o_required,
> uint64_t o_optional)
> {
>@@ -2008,6 +2036,16 @@ static int dl_argv_parse(struct dl *dl, uint64_t o_required,
> dl_arg_inc(dl);
> opts->rate_parent_node = "";
> o_found |= DL_OPT_PORT_FN_RATE_PARENT;
>+ } else if (dl_argv_match(dl, "dry_run") &&
>+ (o_all & DL_OPT_DRY_RUN)) {
>+
>+ if (!dl_kernel_supports_dry_run(dl)) {
>+ pr_err("Kernel does not support dry_run attribute\n");
>+ return -EOPNOTSUPP;
>+ }
>+
>+ dl_arg_inc(dl);
>+ o_found |= DL_OPT_DRY_RUN;
> } else {
> pr_err("Unknown option \"%s\"\n", dl_argv(dl));
> return -EINVAL;
>@@ -2086,6 +2124,8 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
> mnl_attr_put_strz(nlh, DEVLINK_ATTR_RATE_NODE_NAME,
> opts->rate_node_name);
> }
>+ if (opts->present & DL_OPT_DRY_RUN)
>+ mnl_attr_put(nlh, DEVLINK_ATTR_DRY_RUN, 0, NULL);
> if (opts->present & DL_OPT_PORT_TYPE)
> mnl_attr_put_u16(nlh, DEVLINK_ATTR_PORT_TYPE,
> opts->port_type);
>@@ -2284,7 +2324,7 @@ static void cmd_dev_help(void)
> pr_err(" devlink dev reload DEV [ netns { PID | NAME | ID } ]\n");
> pr_err(" [ action { driver_reinit | fw_activate } ] [ limit no_reset ]\n");
> pr_err(" devlink dev info [ DEV ]\n");
>- pr_err(" devlink dev flash DEV file PATH [ component NAME ] [ overwrite SECTION ]\n");
>+ pr_err(" devlink dev flash DEV file PATH [ component NAME ] [ overwrite SECTION ] [ dry_run ]\n");
> }
>
> static bool cmp_arr_last_handle(struct dl *dl, const char *bus_name,
>@@ -3844,7 +3884,8 @@ static int cmd_dev_flash(struct dl *dl)
> NLM_F_REQUEST | NLM_F_ACK);
>
> err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_FLASH_FILE_NAME,
>- DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE);
>+ DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE |
>+ DL_OPT_DRY_RUN);
> if (err)
> return err;
>
>--
>2.36.1
>
Powered by blists - more mailing lists