[PATCH v3] kni: check abi version between kmod and lib

[youcai <omegacoleman@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