From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 0679B1BBA5 for ; Fri, 11 Jan 2019 23:20:10 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jan 2019 14:20:09 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,467,1539673200"; d="scan'208";a="266489239" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga004.jf.intel.com with ESMTP; 11 Jan 2019 14:20:09 -0800 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 11 Jan 2019 14:20:09 -0800 Received: from fmsmsx126.amr.corp.intel.com ([169.254.1.70]) by FMSMSX113.amr.corp.intel.com ([169.254.13.29]) with mapi id 14.03.0415.000; Fri, 11 Jan 2019 14:20:08 -0800 From: "Luse, Paul E" To: "Wiles, Keith" CC: Stephen Hemminger , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 3/6] net/tap: check interface name in kvargs Thread-Index: AQHUqdiP6vswmvuhkkmcpvpJH7XBv6Wq/HUA//+neQk= Date: Fri, 11 Jan 2019 22:20:08 +0000 Message-ID: References: <20190111180659.5972-1-stephen@networkplumber.org> <20190111180659.5972-4-stephen@networkplumber.org>, <14FED5CB-5C2B-452D-BD47-6303EF67F90A@intel.com> In-Reply-To: <14FED5CB-5C2B-452D-BD47-6303EF67F90A@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 3/6] net/tap: check interface name in kvargs 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: Fri, 11 Jan 2019 22:20:11 -0000 Thanks Jim!! -from my iPhone=20 > On Jan 11, 2019, at 11:37 AM, Wiles, Keith wrote: >=20 >=20 >=20 >> On Jan 11, 2019, at 12:06 PM, Stephen Hemminger wrote: >>=20 >> If interface name is passed to remote or iface then check >> the length and for invalid characters. This avoids problems where >> name gets truncated or rejected by kernel. >>=20 >> Signed-off-by: Stephen Hemminger >> --- >> drivers/net/tap/rte_eth_tap.c | 37 +++++++++++++++++++++++++++++++---- >> 1 file changed, 33 insertions(+), 4 deletions(-) >>=20 >> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap= .c >> index d7f77d664502..6a388eed0dd4 100644 >> --- a/drivers/net/tap/rte_eth_tap.c >> +++ b/drivers/net/tap/rte_eth_tap.c >> @@ -37,6 +37,7 @@ >> #include >> #include >> #include >> +#include >>=20 >> #include >> #include >> @@ -1884,6 +1885,23 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, = char *tap_name, >> return -EINVAL; >> } >>=20 >> +/* make sure name is a possible Linux network device name */ >> +static bool is_valid_iface(const char *name) >=20 > I am sorry, but the function name must be on the next line as per the sty= le. I know you do not like it, but that is what the style states. >> +{ >> + if (*name =3D=3D '\0') >> + return false; >> + >> + if (strnlen(name, IFNAMSIZ) =3D=3D IFNAMSIZ) >> + return false; >> + >> + while (*name) { >> + if (*name =3D=3D '/' || *name =3D=3D ':' || isspace(*name)) >> + return false; >> + name++; >> + } >> + return true; >> +} >> + >> static int >> set_interface_name(const char *key __rte_unused, >> const char *value, >> @@ -1891,12 +1909,17 @@ set_interface_name(const char *key __rte_unused, >> { >> char *name =3D (char *)extra_args; >>=20 >> - if (value) >> + if (value) { >> + if (!is_valid_iface(value)) { >> + TAP_LOG(ERR, "TAP invalid remote interface name (%s)", >> + value); >> + return -1; >> + } >> strlcpy(name, value, RTE_ETH_NAME_MAX_LEN); >> - else >> + } else { >> snprintf(name, RTE_ETH_NAME_MAX_LEN, "%s%d", >> DEFAULT_TAP_NAME, tun_unit - 1); >> - >> + } >=20 > This is also a one line else and the style states to remove the {}. >> return 0; >> } >>=20 >> @@ -1907,8 +1930,14 @@ set_remote_iface(const char *key __rte_unused, >> { >> char *name =3D (char *)extra_args; >>=20 >> - if (value) >> + if (value) { >> + if (!is_valid_iface(value)) { >> + TAP_LOG(ERR, "TAP invalid remote interface name (%s)", >> + value); >> + return -1; >> + } >> strlcpy(name, value, RTE_ETH_NAME_MAX_LEN); >> + } >>=20 >> return 0; >> } >> --=20 >> 2.20.1 >>=20 >=20 > I hate having to be the style police, but until we use something to valid= ate the DPDK coding style (as I posted the uncrustify config last month) we= have to keep the style the same or change the DPDK coding standard. >=20 > Regards, > Keith >=20