DPDK patches and discussions
 help / color / mirror / Atom feed
From: youcai <omegacoleman@gmail.com>
To: dev@dpdk.org
Cc: Ray Kinsella <mdr@ashroe.eu>,
	Stephen Hemminger <stephen@networkplumber.org>,
	Ferruh Yigit <ferruh.yigit@xilinx.com>,
	youcai <omegacoleman@gmail.com>
Subject: [PATCH v3] kni: check abi version between kmod and lib
Date: Sun, 24 Apr 2022 16:51:53 +0800	[thread overview]
Message-ID: <20220424085153.925314-1-omegacoleman@gmail.com> (raw)
In-Reply-To: <CAPTjMhh4d6gs93-3fpwhHT5SKUtyq+7FPOV-M=HtHcczzJ3qCA@mail.gmail.com>

KNI ioctl functions copy data from userspace lib, and this interface
of kmod is not compatible indeed. If the user use incompatible rte_kni.ko
bad things happen: sometimes various fields contain garbage value,
sometimes it cause a kmod soft lockup.

Some common distros ship their own rte_kni.ko, so this is likely to
happen.

This patch add abi version checking between userland lib and kmod so
that:

* if kmod ioctl got a wrong abi magic, it refuse to go on
* if userland lib, probed a wrong abi version via newly added ioctl, it
  also refuse to go on

Bugzilla ID: 998

Signed-off-by: youcai <omegacoleman@gmail.com>

---
V3: fix code format issues

V2: use ABI_VERSION instead of a new magic
V2: fix some indent
---
 kernel/linux/kni/kni_misc.c  | 42 ++++++++++++++++++++++++++++++++++++
 kernel/linux/kni/meson.build |  4 ++--
 lib/kni/meson.build          |  1 +
 lib/kni/rte_kni.c            | 18 ++++++++++++++++
 lib/kni/rte_kni_abi.h        | 17 +++++++++++++++
 lib/kni/rte_kni_common.h     |  3 +++
 6 files changed, 83 insertions(+), 2 deletions(-)
 create mode 100644 lib/kni/rte_kni_abi.h

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 780187d8bf..d92500414d 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -17,6 +17,7 @@
 #include <net/netns/generic.h>
 
 #include <rte_kni_common.h>
+#include <rte_kni_abi.h>
 
 #include "compat.h"
 #include "kni_dev.h"
@@ -236,12 +237,26 @@ kni_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
+static int
+kni_check_abi_version_magic(uint16_t abi_version_magic)
+{
+	if (abi_version_magic != RTE_KNI_ABI_VERSION_MAGIC) {
+		pr_err("KNI kmod ABI incompatible with librte_kni -- %u\n",
+				RTE_KNI_ABI_VERSION_FROM_MAGIC(abi_version_magic));
+		return -1;
+	}
+	return 0;
+}
+
 static int
 kni_check_param(struct kni_dev *kni, struct rte_kni_device_info *dev)
 {
 	if (!kni || !dev)
 		return -1;
 
+	if (kni_check_abi_version_magic(dev->abi_version_magic) < 0)
+		return -1;
+
 	/* Check if network name has been used */
 	if (!strncmp(kni->name, dev->name, RTE_KNI_NAMESIZE)) {
 		pr_err("KNI name %s duplicated\n", dev->name);
@@ -301,12 +316,19 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	struct rte_kni_device_info dev_info;
 	struct net_device *net_dev = NULL;
 	struct kni_dev *kni, *dev, *n;
+	uint16_t abi_version_magic;
 
 	pr_info("Creating kni...\n");
 	/* Check the buffer size, to avoid warning */
 	if (_IOC_SIZE(ioctl_num) > sizeof(dev_info))
 		return -EINVAL;
 
+	/* perform abi check ahead of copy, to avoid possible violation */
+	if (copy_from_user(&abi_version_magic, (void *)ioctl_param, sizeof(uint16_t)))
+		return -EFAULT;
+	if (kni_check_abi_version_magic(abi_version_magic) < 0)
+		return -EINVAL;
+
 	/* Copy kni info from user space */
 	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
 		return -EFAULT;
@@ -451,10 +473,17 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
 	int ret = -EINVAL;
 	struct kni_dev *dev, *n;
 	struct rte_kni_device_info dev_info;
+	uint16_t abi_version_magic;
 
 	if (_IOC_SIZE(ioctl_num) > sizeof(dev_info))
 		return -EINVAL;
 
+	/* perform abi check ahead of copy, to avoid possible violation */
+	if (copy_from_user(&abi_version_magic, (void *)ioctl_param, sizeof(uint16_t)))
+		return -EFAULT;
+	if (kni_check_abi_version_magic(abi_version_magic) < 0)
+		return -EINVAL;
+
 	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
 		return -EFAULT;
 
@@ -484,6 +513,16 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
 	return ret;
 }
 
+static int
+kni_ioctl_abi_version(struct net *net, uint32_t ioctl_num,
+		unsigned long ioctl_param)
+{
+	uint16_t abi_version = ABI_VERSION_MAJOR;
+	if (copy_to_user((void *)ioctl_param, &abi_version, sizeof(uint16_t)))
+		return -EFAULT;
+	return 0;
+}
+
 static long
 kni_ioctl(struct file *file, unsigned int ioctl_num, unsigned long ioctl_param)
 {
@@ -505,6 +544,9 @@ kni_ioctl(struct file *file, unsigned int ioctl_num, unsigned long ioctl_param)
 	case _IOC_NR(RTE_KNI_IOCTL_RELEASE):
 		ret = kni_ioctl_release(net, ioctl_num, ioctl_param);
 		break;
+	case _IOC_NR(RTE_KNI_IOCTL_ABI_VERSION):
+		ret = kni_ioctl_abi_version(net, ioctl_num, ioctl_param);
+		break;
 	default:
 		pr_debug("IOCTL default\n");
 		break;
diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
index 4c90069e99..c8cd23acd9 100644
--- a/kernel/linux/kni/meson.build
+++ b/kernel/linux/kni/meson.build
@@ -3,12 +3,12 @@
 
 # For SUSE build check function arguments of ndo_tx_timeout API
 # Ref: https://jira.devtools.intel.com/browse/DPDK-29263
-kmod_cflags = ''
+kmod_cflags = '-DABI_VERSION_MAJOR=@0@'.format(abi_version.split('.')[0])
 file_path = kernel_source_dir + '/include/linux/netdevice.h'
 run_cmd = run_command('grep', 'ndo_tx_timeout', file_path, check: false)
 
 if run_cmd.stdout().contains('txqueue') == true
-   kmod_cflags = '-DHAVE_ARG_TX_QUEUE'
+   kmod_cflags += ' -DHAVE_ARG_TX_QUEUE'
 endif
 
 
diff --git a/lib/kni/meson.build b/lib/kni/meson.build
index 8a71d8ba6f..f22a27b15b 100644
--- a/lib/kni/meson.build
+++ b/lib/kni/meson.build
@@ -14,3 +14,4 @@ endif
 sources = files('rte_kni.c')
 headers = files('rte_kni.h', 'rte_kni_common.h')
 deps += ['ethdev', 'pci']
+cflags += ['-DABI_VERSION_MAJOR=@0@'.format(abi_version.split('.')[0])]
diff --git a/lib/kni/rte_kni.c b/lib/kni/rte_kni.c
index 7971c56bb4..9bdeeb3806 100644
--- a/lib/kni/rte_kni.c
+++ b/lib/kni/rte_kni.c
@@ -22,6 +22,7 @@
 #include <rte_eal_memconfig.h>
 #include <rte_kni_common.h>
 #include "rte_kni_fifo.h"
+#include "rte_kni_abi.h"
 
 #define MAX_MBUF_BURST_NUM            32
 
@@ -113,6 +114,20 @@ rte_kni_init(unsigned int max_kni_ifaces __rte_unused)
 		}
 	}
 
+	uint16_t abi_version;
+	int ret = ioctl(kni_fd, RTE_KNI_IOCTL_ABI_VERSION, &abi_version);
+	if (ret < 0) {
+		RTE_LOG(ERR, KNI, "Cannot verify rte_kni kmod ABI version: ioctl failed\n");
+		return -1;
+	}
+	if (abi_version != ABI_VERSION_MAJOR) {
+		RTE_LOG(ERR, KNI,
+				"rte_kni kmod ABI version mismatch: "
+				"need %" PRIu16 " got %" PRIu16 "\n",
+				ABI_VERSION_MAJOR, abi_version);
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -255,6 +270,7 @@ rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
 		kni->ops.port_id = UINT16_MAX;
 
 	memset(&dev_info, 0, sizeof(dev_info));
+	dev_info.abi_version_magic = RTE_KNI_ABI_VERSION_MAGIC;
 	dev_info.core_id = conf->core_id;
 	dev_info.force_bind = conf->force_bind;
 	dev_info.group_id = conf->group_id;
@@ -409,6 +425,8 @@ rte_kni_release(struct rte_kni *kni)
 	if (!kni)
 		return -1;
 
+	dev_info.abi_version_magic = RTE_KNI_ABI_VERSION_MAGIC;
+
 	kni_list = RTE_TAILQ_CAST(rte_kni_tailq.head, rte_kni_list);
 
 	rte_mcfg_tailq_write_lock();
diff --git a/lib/kni/rte_kni_abi.h b/lib/kni/rte_kni_abi.h
new file mode 100644
index 0000000000..7dde394c72
--- /dev/null
+++ b/lib/kni/rte_kni_abi.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: (BSD-3-Clause OR LGPL-2.1) */
+/*
+ * Copyright(c) 2007-2014 Intel Corporation.
+ */
+
+#ifndef _RTE_KNI_ABI_H_
+#define _RTE_KNI_ABI_H_
+
+#ifndef ABI_VERSION_MAJOR
+#error Need ABI_VERSION_MAJOR being the major part of dpdk/ABI_VERSION
+#endif
+#define RTE_KNI_ABI_VERSION_MAGIC_MASK 0xAAAA
+#define RTE_KNI_ABI_VERSION_MAGIC (((ABI_VERSION_MAJOR) ^ RTE_KNI_ABI_VERSION_MAGIC_MASK))
+#define RTE_KNI_ABI_VERSION_FROM_MAGIC(__magic) (((__magic) ^ RTE_KNI_ABI_VERSION_MAGIC_MASK))
+
+#endif
+
diff --git a/lib/kni/rte_kni_common.h b/lib/kni/rte_kni_common.h
index 8d3ee0fa4f..f9432b742c 100644
--- a/lib/kni/rte_kni_common.h
+++ b/lib/kni/rte_kni_common.h
@@ -102,6 +102,8 @@ struct rte_kni_mbuf {
  */
 
 struct rte_kni_device_info {
+	uint16_t abi_version_magic;
+
 	char name[RTE_KNI_NAMESIZE];  /**< Network device name for KNI */
 
 	phys_addr_t tx_phys;
@@ -139,6 +141,7 @@ struct rte_kni_device_info {
 #define RTE_KNI_IOCTL_TEST    _IOWR(0, 1, int)
 #define RTE_KNI_IOCTL_CREATE  _IOWR(0, 2, struct rte_kni_device_info)
 #define RTE_KNI_IOCTL_RELEASE _IOWR(0, 3, struct rte_kni_device_info)
+#define RTE_KNI_IOCTL_ABI_VERSION _IOWR(0, 4, uint16_t)
 
 #ifdef __cplusplus
 }
-- 
2.35.1


  parent reply	other threads:[~2022-04-24  8:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-21  4:38 Stephen Coleman
2022-04-21 14:16 ` Ray Kinsella
2022-04-21 14:54 ` Stephen Hemminger
2022-04-21 15:40   ` Ray Kinsella
2022-04-21 15:50     ` Stephen Hemminger
2022-04-22  8:46       ` Ray Kinsella
2022-04-22 10:07         ` Stephen Coleman
2022-04-21 16:34 ` [PATCH v2] " youcai
2022-04-24  8:51 ` youcai [this message]
2022-04-24 10:35   ` Stephen Coleman
2023-07-04  2:56 ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220424085153.925314-1-omegacoleman@gmail.com \
    --to=omegacoleman@gmail.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@xilinx.com \
    --cc=mdr@ashroe.eu \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).