From: Harman Kalra Date: Mon, 1 Nov 2021 17:53:34 +0000 (+0530) Subject: drivers: check interrupt file descriptor validity X-Git-Url: http://git.droids-corp.org/?a=commitdiff_plain;h=aedd054c5c2a8565775539e7fb4e8b4e6e95f1ac;p=dpdk.git drivers: check interrupt file descriptor validity This patch fixes coverity issue by adding a check for negative value to avoid bad bit shift operation and other invalid use of file descriptors. Coverity issue: 373717, 373697, 373685 Coverity issue: 373723, 373720, 373719, 373718, 373715, 373714, 373713 Coverity issue: 373710, 373707, 373706, 373705, 373704, 373701, 373700 Coverity issue: 373698, 373695, 373692, 373690, 373689 Coverity issue: 373722, 373721, 373709, 373702, 373696 Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle") Signed-off-by: Harman Kalra Acked-by: Haiyue Wang Acked-by: David Marchand --- diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c index 2ee5d04672..d52125e49b 100644 --- a/drivers/bus/pci/linux/pci_uio.c +++ b/drivers/bus/pci/linux/pci_uio.c @@ -37,6 +37,9 @@ pci_uio_read_config(const struct rte_intr_handle *intr_handle, { int uio_cfg_fd = rte_intr_dev_fd_get(intr_handle); + if (uio_cfg_fd < 0) + return -1; + return pread(uio_cfg_fd, buf, len, offset); } @@ -46,6 +49,9 @@ pci_uio_write_config(const struct rte_intr_handle *intr_handle, { int uio_cfg_fd = rte_intr_dev_fd_get(intr_handle); + if (uio_cfg_fd < 0) + return -1; + return pwrite(uio_cfg_fd, buf, len, offset); } diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c index edcee92556..1a5e7c2d2a 100644 --- a/drivers/bus/pci/linux/pci_vfio.c +++ b/drivers/bus/pci/linux/pci_vfio.c @@ -49,6 +49,9 @@ pci_vfio_read_config(const struct rte_intr_handle *intr_handle, { int vfio_dev_fd = rte_intr_dev_fd_get(intr_handle); + if (vfio_dev_fd < 0) + return -1; + return pread64(vfio_dev_fd, buf, len, VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) + offs); } @@ -59,6 +62,9 @@ pci_vfio_write_config(const struct rte_intr_handle *intr_handle, { int vfio_dev_fd = rte_intr_dev_fd_get(intr_handle); + if (vfio_dev_fd < 0) + return -1; + return pwrite64(vfio_dev_fd, buf, len, VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) + offs); } @@ -1012,6 +1018,9 @@ pci_vfio_unmap_resource_primary(struct rte_pci_device *dev) } #endif + if (rte_intr_fd_get(dev->intr_handle) < 0) + return -1; + if (close(rte_intr_fd_get(dev->intr_handle)) < 0) { RTE_LOG(INFO, EAL, "Error when closing eventfd file descriptor for %s\n", pci_addr); @@ -1019,6 +1028,9 @@ pci_vfio_unmap_resource_primary(struct rte_pci_device *dev) } vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle); + if (vfio_dev_fd < 0) + return -1; + if (pci_vfio_set_bus_master(vfio_dev_fd, false)) { RTE_LOG(ERR, EAL, "%s cannot unset bus mastering for PCI device!\n", pci_addr); @@ -1062,6 +1074,9 @@ pci_vfio_unmap_resource_secondary(struct rte_pci_device *dev) loc->domain, loc->bus, loc->devid, loc->function); vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle); + if (vfio_dev_fd < 0) + return -1; + ret = rte_vfio_release_device(rte_pci_get_sysfs_path(), pci_addr, vfio_dev_fd); if (ret < 0) { @@ -1114,6 +1129,9 @@ pci_vfio_ioport_read(struct rte_pci_ioport *p, const struct rte_intr_handle *intr_handle = p->dev->intr_handle; int vfio_dev_fd = rte_intr_dev_fd_get(intr_handle); + if (vfio_dev_fd < 0) + return; + if (pread64(vfio_dev_fd, data, len, p->base + offset) <= 0) RTE_LOG(ERR, EAL, @@ -1128,6 +1146,9 @@ pci_vfio_ioport_write(struct rte_pci_ioport *p, const struct rte_intr_handle *intr_handle = p->dev->intr_handle; int vfio_dev_fd = rte_intr_dev_fd_get(intr_handle); + if (vfio_dev_fd < 0) + return; + if (pwrite64(vfio_dev_fd, data, len, p->base + offset) <= 0) RTE_LOG(ERR, EAL, diff --git a/drivers/bus/pci/pci_common_uio.c b/drivers/bus/pci/pci_common_uio.c index 244c9a8940..76c661f054 100644 --- a/drivers/bus/pci/pci_common_uio.c +++ b/drivers/bus/pci/pci_common_uio.c @@ -233,7 +233,8 @@ pci_uio_unmap_resource(struct rte_pci_device *dev) rte_free(uio_res); /* close fd if in primary process */ - close(rte_intr_fd_get(dev->intr_handle)); + if (rte_intr_fd_get(dev->intr_handle) >= 0) + close(rte_intr_fd_get(dev->intr_handle)); uio_cfg_fd = rte_intr_dev_fd_get(dev->intr_handle); if (uio_cfg_fd >= 0) { close(uio_cfg_fd); diff --git a/drivers/bus/vmbus/linux/vmbus_uio.c b/drivers/bus/vmbus/linux/vmbus_uio.c index 9c5c1aeca3..5db70f8e0d 100644 --- a/drivers/bus/vmbus/linux/vmbus_uio.c +++ b/drivers/bus/vmbus/linux/vmbus_uio.c @@ -30,7 +30,8 @@ static void *vmbus_map_addr; /* Control interrupts */ void vmbus_uio_irq_control(struct rte_vmbus_device *dev, int32_t onoff) { - if (write(rte_intr_fd_get(dev->intr_handle), &onoff, + if ((rte_intr_fd_get(dev->intr_handle) < 0) || + write(rte_intr_fd_get(dev->intr_handle), &onoff, sizeof(onoff)) < 0) { VMBUS_LOG(ERR, "cannot write to %d:%s", rte_intr_fd_get(dev->intr_handle), @@ -43,6 +44,9 @@ int vmbus_uio_irq_read(struct rte_vmbus_device *dev) int32_t count; int cc; + if (rte_intr_fd_get(dev->intr_handle) < 0) + return -1; + cc = read(rte_intr_fd_get(dev->intr_handle), &count, sizeof(count)); if (cc < (int)sizeof(count)) { diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c index 336296d6a8..882a24f869 100644 --- a/drivers/bus/vmbus/vmbus_common_uio.c +++ b/drivers/bus/vmbus/vmbus_common_uio.c @@ -258,7 +258,9 @@ vmbus_uio_unmap_resource(struct rte_vmbus_device *dev) rte_free(uio_res); /* close fd if in primary process */ - close(rte_intr_fd_get(dev->intr_handle)); + if (rte_intr_fd_get(dev->intr_handle) >= 0) + close(rte_intr_fd_get(dev->intr_handle)); + if (rte_intr_dev_fd_get(dev->intr_handle) >= 0) { close(rte_intr_dev_fd_get(dev->intr_handle)); rte_intr_dev_fd_set(dev->intr_handle, -1); diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c index b9bf9d2966..e49f765434 100644 --- a/drivers/net/dpaa/dpaa_ethdev.c +++ b/drivers/net/dpaa/dpaa_ethdev.c @@ -371,6 +371,9 @@ static void dpaa_interrupt_handler(void *param) dpaa_dev = container_of(rdev, struct rte_dpaa_device, device); intr_handle = dpaa_dev->intr_handle; + if (rte_intr_fd_get(intr_handle) < 0) + return; + bytes_read = read(rte_intr_fd_get(intr_handle), &buf, sizeof(uint64_t)); if (bytes_read < 0) diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index d0e2bc9814..3ee16c15fe 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -5195,7 +5195,7 @@ eth_igb_assign_msix_vector(struct e1000_hw *hw, int8_t direction, static void eth_igb_configure_msix_intr(struct rte_eth_dev *dev) { - int queue_id; + int queue_id, nb_efd; uint32_t tmpval, regval, intr_mask; struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -5244,9 +5244,11 @@ eth_igb_configure_msix_intr(struct rte_eth_dev *dev) E1000_WRITE_REG(hw, E1000_GPIE, E1000_GPIE_MSIX_MODE | E1000_GPIE_PBA | E1000_GPIE_EIAME | E1000_GPIE_NSICR); - intr_mask = - RTE_LEN2MASK(rte_intr_nb_efd_get(intr_handle), - uint32_t) << misc_shift; + nb_efd = rte_intr_nb_efd_get(intr_handle); + if (nb_efd < 0) + return; + + intr_mask = RTE_LEN2MASK(nb_efd, uint32_t) << misc_shift; if (dev->data->dev_conf.intr_conf.lsc != 0) intr_mask |= (1 << IGB_MSIX_OTHER_INTR_VEC); @@ -5264,8 +5266,11 @@ eth_igb_configure_msix_intr(struct rte_eth_dev *dev) /* use EIAM to auto-mask when MSI-X interrupt * is asserted, this saves a register write for every interrupt */ - intr_mask = RTE_LEN2MASK(rte_intr_nb_efd_get(intr_handle), - uint32_t) << misc_shift; + nb_efd = rte_intr_nb_efd_get(intr_handle); + if (nb_efd < 0) + return; + + intr_mask = RTE_LEN2MASK(nb_efd, uint32_t) << misc_shift; if (dev->data->dev_conf.intr_conf.lsc != 0) intr_mask |= (1 << IGB_MSIX_OTHER_INTR_VEC); diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c index 3e2bf14b94..a1f1a9772b 100644 --- a/drivers/net/igc/igc_ethdev.c +++ b/drivers/net/igc/igc_ethdev.c @@ -727,7 +727,7 @@ igc_configure_msix_intr(struct rte_eth_dev *dev) uint32_t vec = IGC_MISC_VEC_ID; uint32_t base = IGC_MISC_VEC_ID; uint32_t misc_shift = 0; - int i; + int i, nb_efd; /* won't configure msix register if no mapping is done * between intr vector and event fd @@ -745,8 +745,12 @@ igc_configure_msix_intr(struct rte_eth_dev *dev) IGC_WRITE_REG(hw, IGC_GPIE, IGC_GPIE_MSIX_MODE | IGC_GPIE_PBA | IGC_GPIE_EIAME | IGC_GPIE_NSICR); - intr_mask = RTE_LEN2MASK(rte_intr_nb_efd_get(intr_handle), - uint32_t) << misc_shift; + + nb_efd = rte_intr_nb_efd_get(intr_handle); + if (nb_efd < 0) + return; + + intr_mask = RTE_LEN2MASK(nb_efd, uint32_t) << misc_shift; if (dev->data->dev_conf.intr_conf.lsc) intr_mask |= (1u << IGC_MSIX_OTHER_INTR_VEC); @@ -802,6 +806,7 @@ igc_rxq_interrupt_setup(struct rte_eth_dev *dev) struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev); struct rte_intr_handle *intr_handle = pci_dev->intr_handle; int misc_shift = rte_intr_allow_others(intr_handle) ? 1 : 0; + int nb_efd; /* won't configure msix register if no mapping is done * between intr vector and event fd @@ -809,8 +814,11 @@ igc_rxq_interrupt_setup(struct rte_eth_dev *dev) if (!rte_intr_dp_is_en(intr_handle)) return; - mask = RTE_LEN2MASK(rte_intr_nb_efd_get(intr_handle), uint32_t) - << misc_shift; + nb_efd = rte_intr_nb_efd_get(intr_handle); + if (nb_efd < 0) + return; + + mask = RTE_LEN2MASK(nb_efd, uint32_t) << misc_shift; IGC_WRITE_REG(hw, IGC_EIMS, mask); } diff --git a/drivers/net/memif/memif_socket.c b/drivers/net/memif/memif_socket.c index d48c3685d9..079cf01269 100644 --- a/drivers/net/memif/memif_socket.c +++ b/drivers/net/memif/memif_socket.c @@ -65,6 +65,9 @@ memif_msg_send_from_queue(struct memif_control_channel *cc) if (e == NULL) return 0; + if (rte_intr_fd_get(cc->intr_handle) < 0) + return -1; + size = memif_msg_send(rte_intr_fd_get(cc->intr_handle), &e->msg, e->fd); if (size != sizeof(memif_msg_t)) { @@ -508,7 +511,8 @@ memif_intr_unregister_handler(struct rte_intr_handle *intr_handle, void *arg) struct memif_control_channel *cc = arg; /* close control channel fd */ - close(rte_intr_fd_get(intr_handle)); + if (rte_intr_fd_get(intr_handle) >= 0) + close(rte_intr_fd_get(intr_handle)); /* clear message queue */ while ((elt = TAILQ_FIRST(&cc->msg_queue)) != NULL) { TAILQ_REMOVE(&cc->msg_queue, elt, next); @@ -651,6 +655,9 @@ memif_msg_receive(struct memif_control_channel *cc) mh.msg_control = ctl; mh.msg_controllen = sizeof(ctl); + if (rte_intr_fd_get(cc->intr_handle) < 0) + return -1; + size = recvmsg(rte_intr_fd_get(cc->intr_handle), &mh, 0); if (size != sizeof(memif_msg_t)) { MIF_LOG(DEBUG, "Invalid message size = %zd", size); diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index e4ebabec6a..43d7378329 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -325,7 +325,8 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) } /* consume interrupt */ - if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0) + if (((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0) && + (rte_intr_fd_get(mq->intr_handle) >= 0)) size = read(rte_intr_fd_get(mq->intr_handle), &b, sizeof(b)); @@ -460,7 +461,8 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) } /* consume interrupt */ - if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0) { + if ((rte_intr_fd_get(mq->intr_handle) >= 0) && + ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0)) { uint64_t b; ssize_t size __rte_unused; size = read(rte_intr_fd_get(mq->intr_handle), &b, @@ -680,7 +682,8 @@ no_free_slots: else __atomic_store_n(&ring->tail, slot, __ATOMIC_RELEASE); - if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0) { + if (((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0) && + (rte_intr_fd_get(mq->intr_handle) >= 0)) { a = 1; size = write(rte_intr_fd_get(mq->intr_handle), &a, sizeof(a)); @@ -835,6 +838,9 @@ no_free_slots: /* Send interrupt, if enabled. */ if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0) { uint64_t a = 1; + if (rte_intr_fd_get(mq->intr_handle) < 0) + return -1; + ssize_t size = write(rte_intr_fd_get(mq->intr_handle), &a, sizeof(a)); if (unlikely(size < 0)) { diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c index 37ac18f951..f1b48cae82 100644 --- a/drivers/net/tap/rte_eth_tap.c +++ b/drivers/net/tap/rte_eth_tap.c @@ -1664,8 +1664,9 @@ tap_dev_intr_handler(void *cb_arg) struct rte_eth_dev *dev = cb_arg; struct pmd_internals *pmd = dev->data->dev_private; - tap_nl_recv(rte_intr_fd_get(pmd->intr_handle), - tap_nl_msg_handler, dev); + if (rte_intr_fd_get(pmd->intr_handle) >= 0) + tap_nl_recv(rte_intr_fd_get(pmd->intr_handle), + tap_nl_msg_handler, dev); } static int @@ -1704,8 +1705,10 @@ clean: } } while (true); - tap_nl_final(rte_intr_fd_get(pmd->intr_handle)); - rte_intr_fd_set(pmd->intr_handle, -1); + if (rte_intr_fd_get(pmd->intr_handle) >= 0) { + tap_nl_final(rte_intr_fd_get(pmd->intr_handle)); + rte_intr_fd_set(pmd->intr_handle, -1); + } return 0; } diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c b/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c index cb37ba097c..db971bad48 100644 --- a/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c +++ b/drivers/vdpa/mlx5/mlx5_vdpa_virtq.c @@ -24,6 +24,9 @@ mlx5_vdpa_virtq_handler(void *cb_arg) uint64_t buf; int nbytes; + if (rte_intr_fd_get(virtq->intr_handle) < 0) + return; + do { nbytes = read(rte_intr_fd_get(virtq->intr_handle), &buf, 8);