From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it1-f196.google.com (mail-it1-f196.google.com [209.85.166.196]) by dpdk.org (Postfix) with ESMTP id 6CF57235 for ; Wed, 20 Feb 2019 11:02:56 +0100 (CET) Received: by mail-it1-f196.google.com with SMTP id z131so13863562itf.5 for ; Wed, 20 Feb 2019 02:02:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=yBnLD+iX1c6ypQGct6S4NzCY6u8B+M4lGRwSC5DY8oc=; b=EvQ/XFkbCEigoR2PFCUeOMijs3tTgAlfKccm9/6haiGZi/B9kokwt6ts9tTAWo+fua DRLq4J3PjNvsSX/TWrjKDfhCAoTMM8ap2W/9+eZDxPoxKzKHx9whP95FeVpbDyQr9VfF tctVSebBEhuZGJiPMqZ9G9XBKRIjUmtsRL/lXAt6QK2ZziTyDp4XE5KooZKdjDh6R06w c3Iv3lL8FQ3BMRFjtqpysCJqx4d8r/dVJb/DoOsl6zfACe5FNmtr6vl/xusDoaSJhyWP tLvzguNXLX6FvEbHwCXD6iWG2Il7/pH5oi1UvxRIOsRWX+m6CjSiSqU4ZwRb/fSyifYy gSEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=yBnLD+iX1c6ypQGct6S4NzCY6u8B+M4lGRwSC5DY8oc=; b=KCwy7+BYd7MkDuMilwD8g97yrbTU1dSZhUrGFCUy6IcgOA8RUI85kiOXmbQfofw5/X eHij8F6OUDIbBaTzEuxV09S2hRB10qaBn45l4nikHhkxq2CSn3cCgEDyLNp2ZBA87geh 5L7EGVeW/ShcaedOTgQ4WALz78Nwp3JJU/4zx8riRZt5TdiGzAa7yRTFd9poXXrv82eg yQNJC4pT6uaL+3cvPrzfVROzlGh9NHbIyypROvm3TMjTRx3oSZPA8ShKeIZoYDcxS7xh fbxd3Q8PtqJGMrqnFHdXLpXC9O+rcjr+rtcAoBSnkYpqHdlvYM+VLNXqjKjX41S0Onou 3J/Q== X-Gm-Message-State: AHQUAua2GAhq3toNufK0S1L3dQoq7JTz9eOzNTRqPjuOr1sFKZ8ZbWev MgwoFC5l+znb1Yl2CEC5fF5pJwxJPSF+AWstDu0Xdw== X-Google-Smtp-Source: AHgI3IYENlikK/E9KwQsddxHRjdwBQZP+AIRe18D0JB6aLblnPDTuzBzCdCUsM5y5A4Abj6ysEvcXodtlMsdKfvAyB8= X-Received: by 2002:a5e:a611:: with SMTP id q17mr17901236ioi.17.1550656975725; Wed, 20 Feb 2019 02:02:55 -0800 (PST) MIME-Version: 1.0 References: <20190215103044.10301-1-alejandro.lucero@netronome.com> <42615D86-C8D5-496F-96E2-97887DDB60CB@mellanox.com> In-Reply-To: <42615D86-C8D5-496F-96E2-97887DDB60CB@mellanox.com> From: Alejandro Lucero Date: Wed, 20 Feb 2019 10:02:44 +0000 Message-ID: To: Yongseok Koh Cc: "stable@dpdk.org" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-stable] [PATH 17.11] net/nfp: fix misuse of strlcpy X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Feb 2019 10:02:56 -0000 On Sat, Feb 16, 2019 at 7:49 AM Yongseok Koh wrote: > > On Feb 15, 2019, at 7:30 PM, Alejandro Lucero < > alejandro.lucero@netronome.com> wrote: > > > > Current strlcpy function is doing the wrong thing and as a consequence > > the firmware does not find the symbol requested precluding the right > > NFP initialization. > > Applied to stable/17.11. > > Thanks! > However, can you explain what strlcpy does wrong? > If it is buggy, we should fix it as there are quite a few occurrences, > due to the new gcc. > > Not sure if this is a strlcpy bug. We were in a hurry for fixing this before the release, so I had no time to investigate, just to get it fixed. This code is not present in current PMD code, so I'm not keen to waste any further time on this. However, note the memset for zeroing the buffer before the copy. That was also required sometimes with strncpy because the fw is really picky and without the zeroing it failed with old fw. I tried to just add the zeroing and keeping strlcpy, but it did not work. Although strlcpy ends up adding a "\0", it does that after the length of the copied string, and the length used in the strlcpy is not right. I would say string literals, used for the char *sym, add a null character as well, so even which such a length based on the destination buffer, it should work. So, it does not seem trivial why the firwmare is not getting the symbol string as it requires. > > Thanks, > Yongseok > > > Using strncpy is safe here since the symbol length can never be longer > > than the buffer size where the firmware will get the symbol to work > > with. However, newer compilers do not allow to have the strncpy using > > the source length as the third parameter, so this patch uses instead a > > memcpy call with a previous memset for cleaning up the buffer to be > > used by the firmware. > > > > Fixes: a5d659c2d03f ("net/nfp: replace strncpy by strlcpy") > > > > Signed-off-by: Alejandro Lucero > > --- > > drivers/net/nfp/nfp_nspu.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/nfp/nfp_nspu.c b/drivers/net/nfp/nfp_nspu.c > > index ac5bce3b1..d4abb6c8e 100644 > > --- a/drivers/net/nfp/nfp_nspu.c > > +++ b/drivers/net/nfp/nfp_nspu.c > > @@ -424,7 +424,9 @@ nfp_nspu_set_bar_from_symbl(nspu_desc_t *desc, const > char *symbl, > > if (!sym_buf) > > return -ENOMEM; > > > > - strlcpy(sym_buf, symbl, sizeof(sym_buf)); > > + memset(sym_buf, 0, desc->buf_size); > > + memcpy(sym_buf, symbl, strlen(symbl)); > > + > > ret = nspu_command(desc, NSP_CMD_GET_SYMBOL, 1, 1, sym_buf, > > NFP_SYM_DESC_LEN, strlen(symbl)); > > if (ret) { > > -- > > 2.17.1 > > > >