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 <<a href="mailto:omegacoleman@gmail.com">omegacoleman@gmail.com</a>> > > --- > 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: <a href="https://jira.devtools.intel.com/browse/DPDK-29263">https://jira.devtools.intel.com/browse/DPDK-29263</a> > -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 > >