[v3] kni: check abi version between kmod and lib
Checks
Commit Message
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
Comments
execuse me, one of the Windows check is failing but I didn't find where's
the build log, nor to determine whether it is related.
> 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
>
>
@@ -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;
@@ -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
@@ -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])]
@@ -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();
new file mode 100644
@@ -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
+
@@ -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
}