From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 44C7EA04B6; Tue, 12 Nov 2019 16:21:01 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 220011B994; Tue, 12 Nov 2019 16:19:55 +0100 (CET) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by dpdk.org (Postfix) with ESMTP id 0343E397D for ; Tue, 12 Nov 2019 16:19:45 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1573571985; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wajN2uAgCJdIWhEQ14+65RUv70K0zkILx/BczzGZEjE=; b=H5I0BCB/wC4pNavzuYakGhf8obxZcSsINrIbgOJErviN+NIJIDYwKfonyFriQiyUZUS1VG AwqSRhlseQs/6MUlNgpiVsd/KdlAdVsmy+uuv1J2Hxu0yw9Mf63wVn5Ue7cb5juF9qZUa7 vEh41EybBcgKJwzbNfTkV86obNT94rI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-138-6r3DDABSPzeg5EEfOf_ePw-1; Tue, 12 Nov 2019 10:19:44 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A29BB805837 for ; Tue, 12 Nov 2019 15:19:43 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-39.ams2.redhat.com [10.36.112.39]) by smtp.corp.redhat.com (Postfix) with ESMTP id 833206315E; Tue, 12 Nov 2019 15:19:42 +0000 (UTC) From: Maxime Coquelin To: dev@dpdk.org Cc: Maxime Coquelin Date: Tue, 12 Nov 2019 16:19:35 +0100 Message-Id: <20191112151935.27518-2-maxime.coquelin@redhat.com> In-Reply-To: <20191112151935.27518-1-maxime.coquelin@redhat.com> References: <20191112151935.27518-1-maxime.coquelin@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-MC-Unique: 6r3DDABSPzeg5EEfOf_ePw-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable Subject: [dpdk-dev] [master PATCH v2 2/2] vhost: fix possible denial of service by leaking FDs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" A malicious Vhost-user master could send in loop hand-crafted vhost-user messages containing more file descriptors the vhost-user slave expects. Doing so causes the application using the vhost-user library to run out of FDs. This issue has been assigned CVE-2019-14818 Fixes: 8f972312b8f4 ("vhost: support vhost-user") Signed-off-by: Maxime Coquelin --- lib/librte_vhost/vhost_user.c | 119 ++++++++++++++++++++++++++++++++-- 1 file changed, 115 insertions(+), 4 deletions(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 6d2431e604..049e37c6de 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -83,6 +83,36 @@ static const char *vhost_message_str[VHOST_USER_MAX] =3D= { static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg); static int read_vhost_message(int sockfd, struct VhostUserMsg *msg); =20 +static void +close_msg_fds(struct VhostUserMsg *msg) +{ +=09int i; + +=09for (i =3D 0; i < msg->fd_num; i++) +=09=09close(msg->fds[i]); +} + +/* + * Ensure the expected number of FDs is received, + * close all FDs and return an error if this is not the case. + */ +static int +validate_msg_fds(struct VhostUserMsg *msg, int expected_fds) +{ +=09if (msg->fd_num =3D=3D expected_fds) +=09=09return 0; + +=09RTE_LOG(ERR, VHOST_CONFIG, +=09=09" Expect %d FDs for request %s, received %d\n", +=09=09expected_fds, +=09=09vhost_message_str[msg->request.master], +=09=09msg->fd_num); + +=09close_msg_fds(msg); + +=09return -1; +} + static uint64_t get_blk_size(int fd) { @@ -179,18 +209,25 @@ vhost_backend_cleanup(struct virtio_net *dev) */ static int vhost_user_set_owner(struct virtio_net **pdev __rte_unused, -=09=09=09struct VhostUserMsg *msg __rte_unused, +=09=09=09struct VhostUserMsg *msg, =09=09=09int main_fd __rte_unused) { +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09return RTE_VHOST_MSG_RESULT_OK; } =20 static int vhost_user_reset_owner(struct virtio_net **pdev, -=09=09=09struct VhostUserMsg *msg __rte_unused, +=09=09=09struct VhostUserMsg *msg, =09=09=09int main_fd __rte_unused) { =09struct virtio_net *dev =3D *pdev; + +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09vhost_destroy_device_notify(dev); =20 =09cleanup_device(dev, 0); @@ -208,6 +245,9 @@ vhost_user_get_features(struct virtio_net **pdev, struc= t VhostUserMsg *msg, =09struct virtio_net *dev =3D *pdev; =09uint64_t features =3D 0; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09rte_vhost_driver_get_features(dev->ifname, &features); =20 =09msg->payload.u64 =3D features; @@ -227,6 +267,9 @@ vhost_user_get_queue_num(struct virtio_net **pdev, stru= ct VhostUserMsg *msg, =09struct virtio_net *dev =3D *pdev; =09uint32_t queue_num =3D 0; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09rte_vhost_driver_get_queue_num(dev->ifname, &queue_num); =20 =09msg->payload.u64 =3D (uint64_t)queue_num; @@ -249,6 +292,9 @@ vhost_user_set_features(struct virtio_net **pdev, struc= t VhostUserMsg *msg, =09struct rte_vdpa_device *vdpa_dev; =09int did =3D -1; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09rte_vhost_driver_get_features(dev->ifname, &vhost_features); =09if (features & ~vhost_features) { =09=09RTE_LOG(ERR, VHOST_CONFIG, @@ -331,6 +377,9 @@ vhost_user_set_vring_num(struct virtio_net **pdev, =09struct virtio_net *dev =3D *pdev; =09struct vhost_virtqueue *vq =3D dev->virtqueue[msg->payload.state.index]= ; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09vq->size =3D msg->payload.state.num; =20 =09/* VIRTIO 1.0, 2.4 Virtqueues says: @@ -718,6 +767,9 @@ vhost_user_set_vring_addr(struct virtio_net **pdev, str= uct VhostUserMsg *msg, =09struct vhost_vring_addr *addr =3D &msg->payload.addr; =09bool access_ok; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09if (dev->mem =3D=3D NULL) =09=09return RTE_VHOST_MSG_RESULT_ERR; =20 @@ -759,6 +811,9 @@ vhost_user_set_vring_base(struct virtio_net **pdev, =09struct vhost_virtqueue *vq =3D dev->virtqueue[msg->payload.state.index]= ; =09uint64_t val =3D msg->payload.state.num; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09if (vq_is_packed(dev)) { =09=09/* =09=09 * Bit[0:14]: avail index @@ -920,6 +975,9 @@ vhost_user_set_mem_table(struct virtio_net **pdev, stru= ct VhostUserMsg *msg, =09int populate; =09int fd; =20 +=09if (validate_msg_fds(msg, memory->nregions) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09if (memory->nregions > VHOST_MEMORY_MAX_NREGIONS) { =09=09RTE_LOG(ERR, VHOST_CONFIG, =09=09=09"too many memory regions (%u)\n", memory->nregions); @@ -930,8 +988,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, stru= ct VhostUserMsg *msg, =09=09RTE_LOG(INFO, VHOST_CONFIG, =09=09=09"(%d) memory regions not changed\n", dev->vid); =20 -=09=09for (i =3D 0; i < memory->nregions; i++) -=09=09=09close(msg->fds[i]); +=09=09close_msg_fds(msg); =20 =09=09return RTE_VHOST_MSG_RESULT_OK; =09} @@ -1074,6 +1131,10 @@ vhost_user_set_mem_table(struct virtio_net **pdev, s= truct VhostUserMsg *msg, =09=09=09=09"Failed to read qemu ack on postcopy set-mem-table\n"); =09=09=09goto err_mmap; =09=09} + +=09=09if (validate_msg_fds(&ack_msg, 0) !=3D 0) +=09=09=09goto err_mmap; + =09=09if (ack_msg.request.master !=3D VHOST_USER_SET_MEM_TABLE) { =09=09=09RTE_LOG(ERR, VHOST_CONFIG, =09=09=09=09"Bad qemu ack on postcopy set-mem-table (%d)\n", @@ -1194,6 +1255,9 @@ vhost_user_set_vring_call(struct virtio_net **pdev, s= truct VhostUserMsg *msg, =09struct vhost_vring_file file; =09struct vhost_virtqueue *vq; =20 +=09if (validate_msg_fds(msg, 1) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09file.index =3D msg->payload.u64 & VHOST_USER_VRING_IDX_MASK; =09if (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) =09=09file.fd =3D VIRTIO_INVALID_EVENTFD; @@ -1215,6 +1279,9 @@ static int vhost_user_set_vring_err(struct virtio_net= **pdev __rte_unused, =09=09=09struct VhostUserMsg *msg, =09=09=09int main_fd __rte_unused) { +=09if (validate_msg_fds(msg, 1) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09if (!(msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK)) =09=09close(msg->fds[0]); =09RTE_LOG(INFO, VHOST_CONFIG, "not implemented\n"); @@ -1230,6 +1297,9 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, s= truct VhostUserMsg *msg, =09struct vhost_vring_file file; =09struct vhost_virtqueue *vq; =20 +=09if (validate_msg_fds(msg, 1) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09file.index =3D msg->payload.u64 & VHOST_USER_VRING_IDX_MASK; =09if (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) =09=09file.fd =3D VIRTIO_INVALID_EVENTFD; @@ -1286,6 +1356,9 @@ vhost_user_get_vring_base(struct virtio_net **pdev, =09struct vhost_virtqueue *vq =3D dev->virtqueue[msg->payload.state.index]= ; =09uint64_t val; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09/* We have to stop the queue (virtio) if it is running. */ =09vhost_destroy_device_notify(dev); =20 @@ -1361,6 +1434,9 @@ vhost_user_set_vring_enable(struct virtio_net **pdev, =09struct rte_vdpa_device *vdpa_dev; =09int did =3D -1; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09RTE_LOG(INFO, VHOST_CONFIG, =09=09"set queue enable: %d to qp idx: %d\n", =09=09enable, index); @@ -1391,6 +1467,9 @@ vhost_user_get_protocol_features(struct virtio_net **= pdev, =09struct virtio_net *dev =3D *pdev; =09uint64_t features, protocol_features; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09rte_vhost_driver_get_features(dev->ifname, &features); =09rte_vhost_driver_get_protocol_features(dev->ifname, &protocol_features)= ; =20 @@ -1419,6 +1498,9 @@ vhost_user_set_protocol_features(struct virtio_net **= pdev, =09uint64_t protocol_features =3D msg->payload.u64; =09uint64_t slave_protocol_features =3D 0; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09rte_vhost_driver_get_protocol_features(dev->ifname, =09=09=09&slave_protocol_features); =09if (protocol_features & ~slave_protocol_features) { @@ -1445,6 +1527,9 @@ vhost_user_set_log_base(struct virtio_net **pdev, str= uct VhostUserMsg *msg, =09uint64_t size, off; =09void *addr; =20 +=09if (validate_msg_fds(msg, 1) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09if (fd < 0) { =09=09RTE_LOG(ERR, VHOST_CONFIG, "invalid log fd: %d\n", fd); =09=09return RTE_VHOST_MSG_RESULT_ERR; @@ -1508,6 +1593,9 @@ static int vhost_user_set_log_fd(struct virtio_net **= pdev __rte_unused, =09=09=09struct VhostUserMsg *msg, =09=09=09int main_fd __rte_unused) { +=09if (validate_msg_fds(msg, 1) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09close(msg->fds[0]); =09RTE_LOG(INFO, VHOST_CONFIG, "not implemented.\n"); =20 @@ -1531,6 +1619,9 @@ vhost_user_send_rarp(struct virtio_net **pdev, struct= VhostUserMsg *msg, =09struct rte_vdpa_device *vdpa_dev; =09int did =3D -1; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09RTE_LOG(DEBUG, VHOST_CONFIG, =09=09":: mac: %02x:%02x:%02x:%02x:%02x:%02x\n", =09=09mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); @@ -1558,6 +1649,10 @@ vhost_user_net_set_mtu(struct virtio_net **pdev, str= uct VhostUserMsg *msg, =09=09=09int main_fd __rte_unused) { =09struct virtio_net *dev =3D *pdev; + +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09if (msg->payload.u64 < VIRTIO_MIN_MTU || =09=09=09msg->payload.u64 > VIRTIO_MAX_MTU) { =09=09RTE_LOG(ERR, VHOST_CONFIG, "Invalid MTU size (%"PRIu64")\n", @@ -1578,6 +1673,9 @@ vhost_user_set_req_fd(struct virtio_net **pdev, struc= t VhostUserMsg *msg, =09struct virtio_net *dev =3D *pdev; =09int fd =3D msg->fds[0]; =20 +=09if (validate_msg_fds(msg, 1) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09if (fd < 0) { =09=09RTE_LOG(ERR, VHOST_CONFIG, =09=09=09=09"Invalid file descriptor for slave channel (%d)\n", @@ -1663,6 +1761,9 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, struct= VhostUserMsg *msg, =09uint16_t i; =09uint64_t vva, len; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09switch (imsg->type) { =09case VHOST_IOTLB_UPDATE: =09=09len =3D imsg->size; @@ -1709,6 +1810,9 @@ vhost_user_set_postcopy_advise(struct virtio_net **pd= ev, #ifdef RTE_LIBRTE_VHOST_POSTCOPY =09struct uffdio_api api_struct; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09dev->postcopy_ufd =3D syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK)= ; =20 =09if (dev->postcopy_ufd =3D=3D -1) { @@ -1744,6 +1848,9 @@ vhost_user_set_postcopy_listen(struct virtio_net **pd= ev, { =09struct virtio_net *dev =3D *pdev; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09if (dev->mem && dev->mem->nregions) { =09=09RTE_LOG(ERR, VHOST_CONFIG, =09=09=09"Regions already registered at postcopy-listen\n"); @@ -1760,6 +1867,9 @@ vhost_user_postcopy_end(struct virtio_net **pdev, str= uct VhostUserMsg *msg, { =09struct virtio_net *dev =3D *pdev; =20 +=09if (validate_msg_fds(msg, 0) !=3D 0) +=09=09return RTE_VHOST_MSG_RESULT_ERR; + =09dev->postcopy_listening =3D 0; =09if (dev->postcopy_ufd >=3D 0) { =09=09close(dev->postcopy_ufd); @@ -2112,6 +2222,7 @@ vhost_user_msg_handler(int vid, int fd) =09if (!handled) { =09=09RTE_LOG(ERR, VHOST_CONFIG, =09=09=09"vhost message (req: %d) was not handled.\n", request); +=09=09close_msg_fds(&msg); =09=09ret =3D RTE_VHOST_MSG_RESULT_ERR; =09} =20 --=20 2.21.0