From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id BD4527CDA; Mon, 29 Apr 2019 16:05:35 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Apr 2019 07:05:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,409,1549958400"; d="scan'208";a="168959429" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.113]) ([10.237.220.113]) by fmsmga001.fm.intel.com with ESMTP; 29 Apr 2019 07:05:33 -0700 To: "Wiles, Keith" , "Lipiec, Herakliusz" Cc: dpdk-dev , "rasland@mellanox.com" , "stable@dpdk.org" References: <20190425164700.30948-1-herakliusz.lipiec@intel.com> <20190425171702.933-1-herakliusz.lipiec@intel.com> <7E00796C-D91F-47C4-B957-8561FC26F0E5@intel.com> From: "Burakov, Anatoly" Message-ID: <45415448-f01d-8b5d-b893-e1fb1f802fdf@intel.com> Date: Mon, 29 Apr 2019 15:05:31 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <7E00796C-D91F-47C4-B957-8561FC26F0E5@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] net/tap: fix potential buffer overrun 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: , X-List-Received-Date: Mon, 29 Apr 2019 14:05:36 -0000 On 29-Apr-19 2:58 PM, Wiles, Keith wrote: > > >> On Apr 25, 2019, at 10:17 AM, Lipiec, Herakliusz wrote: >> >> When secondary to primary process synchronization occours >> there is no check for number of fds which could cause buffer overrun. >> >> Bugzilla ID: 252 >> Fixes: c9aa56edec8e ("net/tap: access primary process queues from secondary") >> Cc: rasland@mellanox.com >> Cc: stable@dpdk.org >> >> Signed-off-by: Herakliusz Lipiec >> --- >> drivers/net/tap/rte_eth_tap.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c >> index e9fda8cf6..4a2ef5ce7 100644 >> --- a/drivers/net/tap/rte_eth_tap.c >> +++ b/drivers/net/tap/rte_eth_tap.c >> @@ -2111,6 +2111,10 @@ tap_mp_attach_queues(const char *port_name, struct rte_eth_dev *dev) >> TAP_LOG(DEBUG, "Received IPC reply for %s", reply_param->port_name); >> >> /* Attach the queues from received file descriptors */ >> + if (reply_param->rxq_count + reply_param->txq_count != reply->num_fds) { >> + TAP_LOG(ERR, "Unexpected number of fds received"); >> + return -1; >> + } > > This check is reasonable, but why is this being done on the receive side and not checked on the send side. There may need to be a check for num_fds being zero or greater than 8 as that is the limit to the number of FDs that can be handle by the IPC. It is done below on the send side as well. This check is for sanity-checking external input. It's a socket, so anything (with matching UID) can write to it. -- Thanks, Anatoly From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 6AD22A0679 for ; Mon, 29 Apr 2019 16:05:38 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 649361B10C; Mon, 29 Apr 2019 16:05:37 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id BD4527CDA; Mon, 29 Apr 2019 16:05:35 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Apr 2019 07:05:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,409,1549958400"; d="scan'208";a="168959429" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.113]) ([10.237.220.113]) by fmsmga001.fm.intel.com with ESMTP; 29 Apr 2019 07:05:33 -0700 To: "Wiles, Keith" , "Lipiec, Herakliusz" Cc: dpdk-dev , "rasland@mellanox.com" , "stable@dpdk.org" References: <20190425164700.30948-1-herakliusz.lipiec@intel.com> <20190425171702.933-1-herakliusz.lipiec@intel.com> <7E00796C-D91F-47C4-B957-8561FC26F0E5@intel.com> From: "Burakov, Anatoly" Message-ID: <45415448-f01d-8b5d-b893-e1fb1f802fdf@intel.com> Date: Mon, 29 Apr 2019 15:05:31 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <7E00796C-D91F-47C4-B957-8561FC26F0E5@intel.com> Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] net/tap: fix potential buffer overrun 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" Message-ID: <20190429140531.itWJzwMERPIhfxuh_xQe_3wMStS3jfZ1v6-Zd9jQ6PM@z> On 29-Apr-19 2:58 PM, Wiles, Keith wrote: > > >> On Apr 25, 2019, at 10:17 AM, Lipiec, Herakliusz wrote: >> >> When secondary to primary process synchronization occours >> there is no check for number of fds which could cause buffer overrun. >> >> Bugzilla ID: 252 >> Fixes: c9aa56edec8e ("net/tap: access primary process queues from secondary") >> Cc: rasland@mellanox.com >> Cc: stable@dpdk.org >> >> Signed-off-by: Herakliusz Lipiec >> --- >> drivers/net/tap/rte_eth_tap.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c >> index e9fda8cf6..4a2ef5ce7 100644 >> --- a/drivers/net/tap/rte_eth_tap.c >> +++ b/drivers/net/tap/rte_eth_tap.c >> @@ -2111,6 +2111,10 @@ tap_mp_attach_queues(const char *port_name, struct rte_eth_dev *dev) >> TAP_LOG(DEBUG, "Received IPC reply for %s", reply_param->port_name); >> >> /* Attach the queues from received file descriptors */ >> + if (reply_param->rxq_count + reply_param->txq_count != reply->num_fds) { >> + TAP_LOG(ERR, "Unexpected number of fds received"); >> + return -1; >> + } > > This check is reasonable, but why is this being done on the receive side and not checked on the send side. There may need to be a check for num_fds being zero or greater than 8 as that is the limit to the number of FDs that can be handle by the IPC. It is done below on the send side as well. This check is for sanity-checking external input. It's a socket, so anything (with matching UID) can write to it. -- Thanks, Anatoly