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 EC57541D4F for ; Thu, 23 Feb 2023 10:38:59 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E35B74317F; Thu, 23 Feb 2023 10:38:59 +0100 (CET) Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) by mails.dpdk.org (Postfix) with ESMTP id 2396D43150 for ; Thu, 23 Feb 2023 10:38:59 +0100 (CET) Received: by mail-wm1-f41.google.com with SMTP id k14-20020a05600c1c8e00b003e22107b7ccso2573643wms.0 for ; Thu, 23 Feb 2023 01:38:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=37V6qMwvd78m3acJ/13/T492SlhyDXe5IV7E15Pn3Lo=; b=Bzkmhal0mz+BQSVryTYMglve+gnurlC5Fr1mdvrlFx+9ISm3Ryw5Y1wzo1VGwrEpmR 83dJjA/Bpwm8j8bMxGcmVjBNwpi+4tZIrRppZ72n/zseVS22TbrBPqU+AM237+u5XgD3 UNKFWwa+scvG49WDbPWeOwXGJDvqSYqWb6J6ygNPfVKZcqTu3JbW/H59zOROV0IOu4K+ 9Sypt+0pFaULWDfai4XoDUNRDl26xfFROtjiD/Kb4zIKh77/ui+3rN91QUbOs5lnkwkN 5rD4V/5NP6+t8O4DXMWTQQUI+aCJtim3UpaCAE2pdnyhIQGjVvlTMyDPrCNcSw7Ob5L7 F0IQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=37V6qMwvd78m3acJ/13/T492SlhyDXe5IV7E15Pn3Lo=; b=Tte0lAS+a2tPl615GBI5po9VAxLHtDLgbh+bhTjXtUsGdbX9zryUdksQqKgrL2oX4e 7v3dqDeQKlRv12XqVhahS5qFzasXPc2R7eCtsi5qX0WvnRGG/OUY3VzlIj33Ud/b/ESp AR/48kfaGBTVpkfu/yZHXGWqCA8A3fbjeFmCaxSntJRpdk2Ebm2KklZlF9EbngpO/rKv VTdpBY5Ebg0hdrE92NRS+8SSrGf7JprBVNlx8P5w815/L3qclRykkwBWOcYrnVT/lJem 2rWxeGMJwEoWLnUqzrWhbHa5ja0Zpv7zD2xYGHDfM+XgTlxWu2rZY9q5btGNZUPzD3Me H4Tw== X-Gm-Message-State: AO0yUKUpoV9rFKuGH2wZM8cmyoP7K+PRFlSur0XgGyBuhGTeCa2jmY5U 9HCpEprsgoCqrRMfrLnLqvI= X-Google-Smtp-Source: AK7set+FHT3dz/QYBRiVoBu7oY8mASzV8Nh49KNI9lQRfg9Vd5M66jcnHSPjLCA4+vQqTLWatzFB1g== X-Received: by 2002:a05:600c:340a:b0:3e7:5142:cf17 with SMTP id y10-20020a05600c340a00b003e75142cf17mr6996137wmp.18.1677145138848; Thu, 23 Feb 2023 01:38:58 -0800 (PST) Received: from localhost ([2a01:4b00:d307:1000:f1d3:eb5e:11f4:a7d9]) by smtp.gmail.com with ESMTPSA id m17-20020a05600c3b1100b003e896d953a8sm6351063wms.17.2023.02.23.01.38.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Feb 2023 01:38:58 -0800 (PST) From: luca.boccassi@gmail.com To: Maxime Coquelin Cc: David Marchand , Chenbo Xia , dpdk stable Subject: patch 'vhost: fix possible FD leaks' has been queued to stable release 20.11.8 Date: Thu, 23 Feb 2023 09:36:36 +0000 Message-Id: <20230223093715.3926893-32-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230223093715.3926893-1-luca.boccassi@gmail.com> References: <20230223093715.3926893-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Hi, FYI, your patch has been queued to stable release 20.11.8 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 02/25/23. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Queued patches are on a temporary branch at: https://github.com/bluca/dpdk-stable This queued commit can be viewed at: https://github.com/bluca/dpdk-stable/commit/6a874396c4c6384acd6168bb3768fbcd18b4c3a5 Thanks. Luca Boccassi --- >From 6a874396c4c6384acd6168bb3768fbcd18b4c3a5 Mon Sep 17 00:00:00 2001 From: Maxime Coquelin Date: Tue, 7 Feb 2023 17:22:39 +0100 Subject: [PATCH] vhost: fix possible FD leaks [ upstream commit 585283f9a703fcd994dbf9aca07dea9445319862 ] On failure, read_vhost_message() only closed the message FDs if the header size was unexpected, but there are other cases where it is required. For example in the case the payload size read from the header is greater than the expected maximum payload size. This patch fixes this by closing all messages FDs in all error cases. It also improve error logging by displaying the request name when failure happens if possible. Fixes: bf472259dde6 ("vhost: fix possible denial of service by leaking FDs") Signed-off-by: Maxime Coquelin Signed-off-by: David Marchand Reviewed-by: Chenbo Xia --- lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 93781f4b80..d42ca1d59c 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -2609,30 +2609,37 @@ read_vhost_message(int sockfd, struct VhostUserMsg *msg) ret = read_fd_message(sockfd, (char *)msg, VHOST_USER_HDR_SIZE, msg->fds, VHOST_MEMORY_MAX_NREGIONS, &msg->fd_num); - if (ret <= 0) { - return ret; - } else if (ret != VHOST_USER_HDR_SIZE) { + if (ret <= 0) + goto out; + + if (ret != VHOST_USER_HDR_SIZE) { VHOST_LOG_CONFIG(ERR, "Unexpected header size read\n"); - close_msg_fds(msg); - return -1; + ret = -1; + goto out; } if (msg->size) { if (msg->size > sizeof(msg->payload)) { VHOST_LOG_CONFIG(ERR, "invalid msg size: %d\n", msg->size); - return -1; + ret = -1; + goto out; } ret = read(sockfd, &msg->payload, msg->size); if (ret <= 0) - return ret; + goto out; if (ret != (int)msg->size) { VHOST_LOG_CONFIG(ERR, "read control message failed\n"); - return -1; + ret = -1; + goto out; } } +out: + if (ret <= 0) + close_msg_fds(msg); + return ret; } @@ -2778,6 +2785,7 @@ vhost_user_msg_handler(int vid, int fd) } } + msg.request.master = VHOST_USER_NONE; ret = read_vhost_message(fd, &msg); if (ret <= 0) { if (ret < 0) -- 2.39.1 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2023-02-23 09:36:29.533149201 +0000 +++ 0032-vhost-fix-possible-FD-leaks.patch 2023-02-23 09:36:28.234170087 +0000 @@ -1 +1 @@ -From 585283f9a703fcd994dbf9aca07dea9445319862 Mon Sep 17 00:00:00 2001 +From 6a874396c4c6384acd6168bb3768fbcd18b4c3a5 Mon Sep 17 00:00:00 2001 @@ -5,0 +6,2 @@ +[ upstream commit 585283f9a703fcd994dbf9aca07dea9445319862 ] + @@ -17 +18,0 @@ -Cc: stable@dpdk.org @@ -23,2 +24,2 @@ - lib/vhost/vhost_user.c | 40 ++++++++++++++++++++++++++-------------- - 1 file changed, 26 insertions(+), 14 deletions(-) + lib/librte_vhost/vhost_user.c | 24 ++++++++++++++++-------- + 1 file changed, 16 insertions(+), 8 deletions(-) @@ -26,5 +27,5 @@ -diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c -index b4236820a2..d702d082dd 100644 ---- a/lib/vhost/vhost_user.c -+++ b/lib/vhost/vhost_user.c -@@ -2831,29 +2831,36 @@ read_vhost_message(struct virtio_net *dev, int sockfd, struct vhu_msg_context * +diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c +index 93781f4b80..d42ca1d59c 100644 +--- a/lib/librte_vhost/vhost_user.c ++++ b/lib/librte_vhost/vhost_user.c +@@ -2609,30 +2609,37 @@ read_vhost_message(int sockfd, struct VhostUserMsg *msg) @@ -32,2 +33,2 @@ - ret = read_fd_message(dev->ifname, sockfd, (char *)&ctx->msg, VHOST_USER_HDR_SIZE, - ctx->fds, VHOST_MEMORY_MAX_NREGIONS, &ctx->fd_num); + ret = read_fd_message(sockfd, (char *)msg, VHOST_USER_HDR_SIZE, + msg->fds, VHOST_MEMORY_MAX_NREGIONS, &msg->fd_num); @@ -41,2 +42,2 @@ - VHOST_LOG_CONFIG(dev->ifname, ERR, "Unexpected header size read\n"); -- close_msg_fds(ctx); + VHOST_LOG_CONFIG(ERR, "Unexpected header size read\n"); +- close_msg_fds(msg); @@ -48,4 +49,4 @@ - if (ctx->msg.size) { - if (ctx->msg.size > sizeof(ctx->msg.payload)) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "invalid msg size: %d\n", - ctx->msg.size); + if (msg->size) { + if (msg->size > sizeof(msg->payload)) { + VHOST_LOG_CONFIG(ERR, + "invalid msg size: %d\n", msg->size); @@ -56 +57 @@ - ret = read(sockfd, &ctx->msg.payload, ctx->msg.size); + ret = read(sockfd, &msg->payload, msg->size); @@ -60,2 +61,3 @@ - if (ret != (int)ctx->msg.size) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "read control message failed\n"); + if (ret != (int)msg->size) { + VHOST_LOG_CONFIG(ERR, + "read control message failed\n"); @@ -70 +72 @@ -+ close_msg_fds(ctx); ++ close_msg_fds(msg); @@ -75 +77 @@ -@@ -3031,13 +3038,10 @@ vhost_user_msg_handler(int vid, int fd) +@@ -2778,6 +2785,7 @@ vhost_user_msg_handler(int vid, int fd) @@ -79,28 +81,4 @@ -+ ctx.msg.request.master = VHOST_USER_NONE; - ret = read_vhost_message(dev, fd, &ctx); -- if (ret <= 0) { -- if (ret < 0) -- VHOST_LOG_CONFIG(dev->ifname, ERR, "vhost read message failed\n"); -- else -- VHOST_LOG_CONFIG(dev->ifname, INFO, "vhost peer closed\n"); -- -+ if (ret == 0) { -+ VHOST_LOG_CONFIG(dev->ifname, INFO, "vhost peer closed\n"); - return -1; - } - -@@ -3047,6 +3051,14 @@ vhost_user_msg_handler(int vid, int fd) - else - msg_handler = NULL; - -+ if (ret < 0) { -+ VHOST_LOG_CONFIG(dev->ifname, ERR, "vhost read message %s%s%sfailed\n", -+ msg_handler != NULL ? "for " : "", -+ msg_handler != NULL ? msg_handler->description : "", -+ msg_handler != NULL ? " " : ""); -+ return -1; -+ } -+ - if (msg_handler != NULL && msg_handler->description != NULL) { - if (request != VHOST_USER_IOTLB_MSG) - VHOST_LOG_CONFIG(dev->ifname, INFO, ++ msg.request.master = VHOST_USER_NONE; + ret = read_vhost_message(fd, &msg); + if (ret <= 0) { + if (ret < 0)