From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 10E23A00C3; Thu, 21 Apr 2022 06:38:40 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E4E8C410E1; Thu, 21 Apr 2022 06:38:39 +0200 (CEST) Received: from mail-il1-f176.google.com (mail-il1-f176.google.com [209.85.166.176]) by mails.dpdk.org (Postfix) with ESMTP id 5B4984068E for ; Thu, 21 Apr 2022 06:38:38 +0200 (CEST) Received: by mail-il1-f176.google.com with SMTP id r11so2355094ila.1 for ; Wed, 20 Apr 2022 21:38:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:from:date:message-id:subject:to:cc; bh=EBMD3uvPbbErbNsnx9FXjMgvykZLFD5Up33gtNaAUoc=; b=KAFSLbRc5O9pQXvCym056MPW0OeM+vFHxDP6uwN+bKbonrPKkNw/+8TyPzAwLhhp7y txhFRR0iZKOrPiIMwGksGIPXanPVd+Ehh9Vy8v2FnWx4Q5Thr/fXy9jY6nOyCjsKvyQJ d/aRvURHrmGUXAauKtYUehAs+yh7lHzzzgymDlfia3C6l6r4LGx5Qmu/8+nqov++A4UQ XySnmv7JhJe4ErfwRopZ4bHef+KsuDKrH/XwyCwFrgTv5CaM9pXrI+Eee+w+E0b77L/Y wA7/W4EMFHhmlD8FWNzN6aJlBNiigLK90N7g2TjXktIIcDBxeVAv9RawrEZohurU697Z ATRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=EBMD3uvPbbErbNsnx9FXjMgvykZLFD5Up33gtNaAUoc=; b=cjVihltUPgeXZH4/gvxRkTFboZsal180BEmh17mVI5VKpg4kLh/R5ovFg3XRIlKcV/ TY9nN4uRt2RgZVRX6S31cBMsmdBH+/gF3hSryb76h8S1mPz3YP1QLlHWqYSkzng5M7OH XBUaqHudx/msUsaJLvha2Z50LrMrN2+VtG/XpH/NKJI7tSFIz0Ec8bXpgnIzl1MYEYje y/g9nr7VlZ86KJqFv/14lm1K1GJ8Fe15YtT9ZkTsx8cE3dp5eI/8v9K+4Mk55NjU9Mmz ItoakaDLaOOrzHgX/mNNSKjn7d5oW7Iwlt5UwX1YQ+F1bUtG0LjHjcBRnDtWATMh7P18 0Pqg== X-Gm-Message-State: AOAM532Ls6MZSMNaI1lZfox4KhqDIgDL9jgiQv6mc2jPYHNqTdHsq9fL zmgPXZs6HiEHAXkoCnP6mUWs0zLfW+PGWO27My9wHA5+WnvC8A== X-Google-Smtp-Source: ABdhPJzDyj7K42VmtuWF2rJ7Bk9WGvZEIyH0EiONNB/RhIhEsEoRDEb7iSuCSruoFIJZbVfA+ST3rkKkY9UbxkjkBPk= X-Received: by 2002:a92:c249:0:b0:2cd:5e07:7725 with SMTP id k9-20020a92c249000000b002cd5e077725mr1256260ilo.227.1650515917656; Wed, 20 Apr 2022 21:38:37 -0700 (PDT) MIME-Version: 1.0 From: Stephen Coleman Date: Thu, 21 Apr 2022 12:38:26 +0800 Message-ID: Subject: kni: check abi version between kmod and lib To: dev@dpdk.org Cc: Ray Kinsella , Ferruh Yigit Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 --- kernel/linux/kni/kni_misc.c | 38 +++++++++++++++++++++++++++++++++++++ lib/kni/rte_kni.c | 16 ++++++++++++++++ lib/kni/rte_kni_common.h | 11 +++++++++++ 3 files changed, 65 insertions(+) diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 780187d8bf..cd9a05d8c1 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -236,12 +236,24 @@ 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 +313,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 +470,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 +510,15 @@ 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 = RTE_KNI_ABI_VERSION; + copy_to_user((void *)ioctl_param, &abi_version, sizeof(uint16_t)); + return 0; +} + static long kni_ioctl(struct file *file, unsigned int ioctl_num, unsigned long ioctl_param) { @@ -505,6 +540,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/lib/kni/rte_kni.c b/lib/kni/rte_kni.c index 7971c56bb4..d7116e582c 100644 --- a/lib/kni/rte_kni.c +++ b/lib/kni/rte_kni.c @@ -113,6 +113,19 @@ 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 != RTE_KNI_ABI_VERSION) { + RTE_LOG(ERR, KNI, + "rte_kni kmod ABI version mismatch: " + "need %" PRIu16 " got %" PRIu16 "\n", RTE_KNI_ABI_VERSION, abi_version); + return -1; + } + return 0; } @@ -255,6 +268,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 +423,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_common.h b/lib/kni/rte_kni_common.h index 8d3ee0fa4f..c353043cb6 100644 --- a/lib/kni/rte_kni_common.h +++ b/lib/kni/rte_kni_common.h @@ -26,6 +26,14 @@ extern "C" { #define RTE_CACHE_LINE_MIN_SIZE 64 +/* + * Ascend this number if ABI is altered between kmod and userland lib + */ +#define RTE_KNI_ABI_VERSION 1 +#define RTE_KNI_ABI_VERSION_MAGIC_MASK 0xAAAA +#define RTE_KNI_ABI_VERSION_MAGIC (((RTE_KNI_ABI_VERSION) ^ RTE_KNI_ABI_VERSION_MAGIC_MASK)) +#define RTE_KNI_ABI_VERSION_FROM_MAGIC(__magic) (((__magic) ^ RTE_KNI_ABI_VERSION_MAGIC_MASK)) + /* * Request id. */ @@ -102,6 +110,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 +149,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.30.1