| 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: Tue, 15 Nov 2022 21:34:23 -0800
From: Saeed Mahameed <saeed@...nel.org>
To: Jeroen de Borst <jeroendb@...gle.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
jesse.brandeburg@...el.com
Subject: Re: [PATCH net-next v3 1/2] gve: Adding a new AdminQ command to
verify driver
On 14 Nov 15:35, Jeroen de Borst wrote:
>Check whether the driver is compatible with the device
>presented.
>
>Signed-off-by: Jeroen de Borst <jeroendb@...gle.com>
>Reviewed-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
>---
> drivers/net/ethernet/google/gve/gve.h | 1 +
> drivers/net/ethernet/google/gve/gve_adminq.c | 19 +++++++
> drivers/net/ethernet/google/gve/gve_adminq.h | 49 ++++++++++++++++++
> drivers/net/ethernet/google/gve/gve_main.c | 52 ++++++++++++++++++++
> 4 files changed, 121 insertions(+)
>
>diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
>index 5655da9cd236..64eb0442c82f 100644
>--- a/drivers/net/ethernet/google/gve/gve.h
>+++ b/drivers/net/ethernet/google/gve/gve.h
>@@ -563,6 +563,7 @@ struct gve_priv {
> u32 adminq_report_stats_cnt;
> u32 adminq_report_link_speed_cnt;
> u32 adminq_get_ptype_map_cnt;
>+ u32 adminq_verify_driver_compatibility_cnt;
>
> /* Global stats */
> u32 interface_up_cnt; /* count of times interface turned up since last reset */
>diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
>index f7621ab672b9..6a12b30a9f87 100644
>--- a/drivers/net/ethernet/google/gve/gve_adminq.c
>+++ b/drivers/net/ethernet/google/gve/gve_adminq.c
>@@ -407,6 +407,9 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
> case GVE_ADMINQ_GET_PTYPE_MAP:
> priv->adminq_get_ptype_map_cnt++;
> break;
>+ case GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY:
>+ priv->adminq_verify_driver_compatibility_cnt++;
>+ break;
> default:
> dev_err(&priv->pdev->dev, "unknown AQ command opcode %d\n", opcode);
> }
>@@ -878,6 +881,22 @@ int gve_adminq_report_stats(struct gve_priv *priv, u64 stats_report_len,
> return gve_adminq_execute_cmd(priv, &cmd);
> }
>
>+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
>+ u64 driver_info_len,
>+ dma_addr_t driver_info_addr)
>+{
>+ union gve_adminq_command cmd;
>+
>+ memset(&cmd, 0, sizeof(cmd));
>+ cmd.opcode = cpu_to_be32(GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY);
>+ cmd.verify_driver_compatibility = (struct gve_adminq_verify_driver_compatibility) {
>+ .driver_info_len = cpu_to_be64(driver_info_len),
>+ .driver_info_addr = cpu_to_be64(driver_info_addr),
>+ };
>+
>+ return gve_adminq_execute_cmd(priv, &cmd);
>+}
>+
> int gve_adminq_report_link_speed(struct gve_priv *priv)
> {
> union gve_adminq_command gvnic_cmd;
>diff --git a/drivers/net/ethernet/google/gve/gve_adminq.h b/drivers/net/ethernet/google/gve/gve_adminq.h
>index 83c0b40cd2d9..b9ee8be73f96 100644
>--- a/drivers/net/ethernet/google/gve/gve_adminq.h
>+++ b/drivers/net/ethernet/google/gve/gve_adminq.h
>@@ -24,6 +24,7 @@ enum gve_adminq_opcodes {
> GVE_ADMINQ_REPORT_STATS = 0xC,
> GVE_ADMINQ_REPORT_LINK_SPEED = 0xD,
> GVE_ADMINQ_GET_PTYPE_MAP = 0xE,
>+ GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY = 0xF,
> };
>
> /* Admin queue status codes */
>@@ -146,6 +147,49 @@ enum gve_sup_feature_mask {
>
> #define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
>
>+#define GVE_VERSION_STR_LEN 128
>+
>+enum gve_driver_capbility {
>+ gve_driver_capability_gqi_qpl = 0,
>+ gve_driver_capability_gqi_rda = 1,
>+ gve_driver_capability_dqo_qpl = 2, /* reserved for future use */
>+ gve_driver_capability_dqo_rda = 3,
>+};
>+
>+#define GVE_CAP1(a) BIT((int)a)
>+#define GVE_CAP2(a) BIT(((int)a) - 64)
>+#define GVE_CAP3(a) BIT(((int)a) - 128)
>+#define GVE_CAP4(a) BIT(((int)a) - 192)
>+
>+#define GVE_DRIVER_CAPABILITY_FLAGS1 \
>+ (GVE_CAP1(gve_driver_capability_gqi_qpl) | \
>+ GVE_CAP1(gve_driver_capability_gqi_rda) | \
>+ GVE_CAP1(gve_driver_capability_dqo_rda))
>+
>+#define GVE_DRIVER_CAPABILITY_FLAGS2 0x0
>+#define GVE_DRIVER_CAPABILITY_FLAGS3 0x0
>+#define GVE_DRIVER_CAPABILITY_FLAGS4 0x0
>+
>+struct gve_driver_info {
>+ u8 os_type; /* 0x01 = Linux */
>+ u8 driver_major;
>+ u8 driver_minor;
>+ u8 driver_sub;
>+ __be32 os_version_major;
>+ __be32 os_version_minor;
>+ __be32 os_version_sub;
>+ __be64 driver_capability_flags[4];
>+ u8 os_version_str1[GVE_VERSION_STR_LEN];
>+ u8 os_version_str2[GVE_VERSION_STR_LEN];
>+};
>+
>+struct gve_adminq_verify_driver_compatibility {
>+ __be64 driver_info_len;
>+ __be64 driver_info_addr;
>+};
>+
>+static_assert(sizeof(struct gve_adminq_verify_driver_compatibility) == 16);
>+
> struct gve_adminq_configure_device_resources {
> __be64 counter_array;
> __be64 irq_db_addr;
>@@ -345,6 +389,8 @@ union gve_adminq_command {
> struct gve_adminq_report_stats report_stats;
> struct gve_adminq_report_link_speed report_link_speed;
> struct gve_adminq_get_ptype_map get_ptype_map;
>+ struct gve_adminq_verify_driver_compatibility
>+ verify_driver_compatibility;
> };
> };
> u8 reserved[64];
>@@ -372,6 +418,9 @@ int gve_adminq_unregister_page_list(struct gve_priv *priv, u32 page_list_id);
> int gve_adminq_set_mtu(struct gve_priv *priv, u64 mtu);
> int gve_adminq_report_stats(struct gve_priv *priv, u64 stats_report_len,
> dma_addr_t stats_report_addr, u64 interval);
>+int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
>+ u64 driver_info_len,
>+ dma_addr_t driver_info_addr);
> int gve_adminq_report_link_speed(struct gve_priv *priv);
>
> struct gve_ptype_lut;
>diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
>index 5a229a01f49d..5b40f9c53196 100644
>--- a/drivers/net/ethernet/google/gve/gve_main.c
>+++ b/drivers/net/ethernet/google/gve/gve_main.c
>@@ -12,6 +12,8 @@
> #include <linux/sched.h>
> #include <linux/timer.h>
> #include <linux/workqueue.h>
>+#include <linux/utsname.h>
>+#include <linux/version.h>
> #include <net/sch_generic.h>
> #include "gve.h"
> #include "gve_dqo.h"
>@@ -30,6 +32,49 @@
> const char gve_version_str[] = GVE_VERSION;
> static const char gve_version_prefix[] = GVE_VERSION_PREFIX;
>
>+static int gve_verify_driver_compatibility(struct gve_priv *priv)
>+{
>+ int err;
>+ struct gve_driver_info *driver_info;
>+ dma_addr_t driver_info_bus;
>+
>+ driver_info = dma_alloc_coherent(&priv->pdev->dev,
>+ sizeof(struct gve_driver_info),
>+ &driver_info_bus, GFP_KERNEL);
>+ if (!driver_info)
>+ return -ENOMEM;
>+
>+ *driver_info = (struct gve_driver_info) {
>+ .os_type = 1, /* Linux */
>+ .os_version_major = cpu_to_be32(LINUX_VERSION_MAJOR),
>+ .os_version_minor = cpu_to_be32(LINUX_VERSION_SUBLEVEL),
>+ .os_version_sub = cpu_to_be32(LINUX_VERSION_PATCHLEVEL),
>+ .driver_capability_flags = {
>+ cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS1),
>+ cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS2),
>+ cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS3),
>+ cpu_to_be64(GVE_DRIVER_CAPABILITY_FLAGS4),
>+ },
>+ };
>+ strscpy(driver_info->os_version_str1, utsname()->release,
>+ sizeof(driver_info->os_version_str1));
>+ strscpy(driver_info->os_version_str2, utsname()->version,
>+ sizeof(driver_info->os_version_str2));
>+
>+ err = gve_adminq_verify_driver_compatibility(priv,
>+ sizeof(struct gve_driver_info),
>+ driver_info_bus);
>+
>+ /* It's ok if the device doesn't support this */
>+ if (err == -EOPNOTSUPP)
>+ err = 0;
>+
I always find these exception interesting so i had to attempt and chase this
error code down to the source and couldn't find it.
I think you meant -ENOTSUPP: which comes from gve_adminq_parse_err()
case GVE_ADMINQ_COMMAND_ERROR_UNIMPLEMENTED:
return -ENOTSUPP;
because there isn't any path in the code that returns -EOPNOTSUPP;
but isn't there any capability that could tell if driver version
compatibility command is supported by device, prior to issuing the command,
so the code can be more explicit.
Powered by blists - more mailing lists