From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-f195.google.com (mail-pf1-f195.google.com [209.85.210.195]) by dpdk.org (Postfix) with ESMTP id 31FE91B942 for ; Fri, 8 Feb 2019 18:35:12 +0100 (CET) Received: by mail-pf1-f195.google.com with SMTP id z9so1992074pfi.2 for ; Fri, 08 Feb 2019 09:35:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=yJ5Oevpz/vRlDczgISBZD7eBm8lUJYliV/X7i1ahcU8=; b=fglY73mvlEElhgDGElmR4OBeMVAs/QPrRWNxtizFNsKkx37SKtoMqLKH57wYKPFr6+ jbFH/wEHDhLgV98p1RFY3KSNSJk9Vada7ovzQIrxmHCVTh95ZGCnT5eW3GWniHvi3MZ0 IKxs95to3w4p1QrvJ6RafTcpblDmEoY3Fh2vv4HUiL1B25jdn4xXZWAvgK5wuAPaF0Hk rAchYVAIWY5l85cHITPbKbjH/XoCS4h+IwUEiX9p0LDtWK7cIc2wrP+TzPKrtl0eAL8b sLngA5fK8DQ0MJEiVI14ZYbNoG0aBruLFNMTw7L/NcjsBqw2yBjWJ4fUbgvKFS1BVCPz T0cw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=yJ5Oevpz/vRlDczgISBZD7eBm8lUJYliV/X7i1ahcU8=; b=YVCn25sdhX7G36CWdA0Ji7stfS1uFtnKhRAVrl7vyu0Tv0h0ezwB4A+MY3qi6XH4pt Rn3/VFuf9NQG+0OTVVhLkVN2X+k67BbPwAK2hFxqloh957uZdYNSlf4t7l9RHC3kz5SS /K5m0olIy6Vpff8Up//NDQZ0LBS3O0SJBzwfj+aO0BR4yLR2IUjZz53Y9lddFwyqN0+d IRl3X0/LM+32ENYAFZpbxWSvTpxw8pYjzgOjIzn6VVJGe/ig9qcu5A8ArhR2juV8JpSY R8AOyfujLH8t4ewnhqLqHWSGY6GPnlv1znAu8C+Fg+sECe5OiaE6qutxlIWCgLLHLCFh bn1Q== X-Gm-Message-State: AHQUAuZYlZEBN64Gq2gyTtv71yJ0oZkyJ9drn/pP2ljqa2oNySSwvxQf ezvl06eCwpOLpQY4E419djBLHDZaxH8= X-Google-Smtp-Source: AHgI3IZuXLecDTHgTvvSzz3APFSJ5Vzl76VZiWLeSPmepTp6oEa/nRztY5GbuuFBwwYozorSAOuVwA== X-Received: by 2002:a63:184a:: with SMTP id 10mr20975146pgy.81.1549647311137; Fri, 08 Feb 2019 09:35:11 -0800 (PST) Received: from hermes.lan (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id w128sm4646572pfw.79.2019.02.08.09.35.10 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 08 Feb 2019 09:35:11 -0800 (PST) Date: Fri, 8 Feb 2019 09:35:02 -0800 From: Stephen Hemminger To: Aaron Conole Cc: Pallantla Poornima , dev@dpdk.org, reshma.pattan@intel.com, ferruh.yigit@intel.com, stable@dpdk.org Message-ID: <20190208093502.7b7047a7@hermes.lan> In-Reply-To: References: <1549632457-15892-1-git-send-email-pallantlax.poornima@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] test: fix sprintf with snprintf 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, 08 Feb 2019 17:35:12 -0000 On Fri, 08 Feb 2019 12:04:22 -0500 Aaron Conole wrote: > Pallantla Poornima writes: > > > sprintf function is not secure as it doesn't check the length of string. > > More secure function snprintf is used. > > > > Fixes: 727909c592 ("app/test: introduce dynamic commands list") > > Cc: stable@dpdk.org > > > > Signed-off-by: Pallantla Poornima > > --- > > test/test/commands.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/test/test/commands.c b/test/test/commands.c > > index 94fbc310e..5aeb35498 100644 > > --- a/test/test/commands.c > > +++ b/test/test/commands.c > > @@ -367,6 +367,8 @@ int commands_init(void) > > struct test_command *t; > > char *commands, *ptr; > > int commands_len = 0; > > + int total_written = 0; > > + int count = 0; > > > > TAILQ_FOREACH(t, &commands_list, next) { > > commands_len += strlen(t->command) + 1; > > @@ -378,7 +380,10 @@ int commands_init(void) > > > > ptr = commands; > > TAILQ_FOREACH(t, &commands_list, next) { > > - ptr += sprintf(ptr, "%s#", t->command); > > + count = snprintf(ptr, commands_len - total_written - 1, "%s#", > > + t->command); > > + ptr += count; > > This code is wrong. From the manpage: > > Upon successful completion, the snprintf() function shall return > the number of bytes that would be written to s had n been > sufficiently large excluding the terminating null byte. > > This code you've placed will improperly increment the number of bytes > taken, since you don't actually check it. > > Additionally, the correct size is calculated in the preceeding blocks, > and then the appropriately sized block is allocated. It doesn't make > any sense to make the change this way. > > If you are intent on changing this code, I suggest something like the > following (completely untested code). The rte_xsprintf() function can > be used in other areas where you're proposing these refactors (but again > see my earlier comments about whether these are actual concerns, or just > 'I dislike the sprintf call'). I agree snprintf is dangerous. Inventing another routine is not really helping much. New code for printing should not be inline (because it isn't performance critical). Why not just rewrite the code in the test to use better string management