From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id DE7E62C17 for ; Tue, 29 Aug 2017 18:33:49 +0200 (CEST) Received: by mail-wm0-f49.google.com with SMTP id t201so1048490wmt.1 for ; Tue, 29 Aug 2017 09:33:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=wA4LU7P/b+NaOPQMSglHR6Gf8/YSGBBauwwYHMILjVI=; b=IxFXl2eyxAe4wcs+gPBwY6RYBNOLWWmpf9CA+GJGjmI0XD9pXnyqiondGEwPk5HRau vZJa3qJd0pKB8S2oathOrw0+CNw0FZXc55bV51R9Rj8ee7I0z5s4tWs+LVOhO4JjuDAn 7E0gmzP8B+1OLnN0qwhMB3JMH+gq4trzDt4HiExLrKWR7Ewj/+OJM/EXKTuJs2Os1vqb ZoZ0FjBPtqAVjbGG7bm5WF9YWpQdJ+2luFkN1HfSeQOC7rJuvW0G0wLQeWeEMihjqWLD YydTZqfKPlNqp6+mrpww8NRhk8e5S5WbigZc8dbWf87COyFcoYTawFTcIauScFkWX/Ph ofgQ== 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:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=wA4LU7P/b+NaOPQMSglHR6Gf8/YSGBBauwwYHMILjVI=; b=KQC2lDfqbs+9ywQzgtd2mnuiUaECf3bxxub4jlfwEFAP3qsSth2kbN8ZWBMcfvzZs5 RbFnvrN1a5w34xXHI29d0MJ8aMIeIMLbcW7U42N1/15M++nDgBbU4LAfYymVIT8Oiamp PZX7Z/BQKLmYDT215nVUlJHRiODtZx35SMHhqzHO3kPNp6JF85WsFAjQhhJqn67W1wTg E/ZH5UfvrbS4JPJyufm3snmZf5S52wMP44R5vgJHYDyR+oE26tOkJCmClqMbUYVwi6QS eh8o8SiFA+j4ZpZPzsdkTDshJdMqMW0syi2HHPA1C4uHsZUIFLiufjVygAE14bKuoA5N wynw== X-Gm-Message-State: AHYfb5hmUHP2z7PtPYGMp+AXLpWM+FRMHslM6myrR1mcmSqL2ngPrvot bLI0IkYIUfa1I8W2 X-Received: by 10.28.1.209 with SMTP id 200mr131158wmb.49.1504024429544; Tue, 29 Aug 2017 09:33:49 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id a89sm4185990wrc.24.2017.08.29.09.33.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 29 Aug 2017 09:33:48 -0700 (PDT) Date: Tue, 29 Aug 2017 18:33:39 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Matan Azrad Cc: dev@dpdk.org, Raslan Darawsheh , stable@dpdk.org Message-ID: <20170829163339.GP8124@bidouze.vm.6wind.com> References: <1504018748-4766-1-git-send-email-matan@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1504018748-4766-1-git-send-email-matan@mellanox.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-stable] [PATCH] net/failsafe: fix exec parameter parsing error flow 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: Tue, 29 Aug 2017 16:33:50 -0000 Hi Matan, On Tue, Aug 29, 2017 at 05:59:08PM +0300, Matan Azrad wrote: > The corrupted code returns success value in case of the > execution process output stream is empty(EOF). > It causes to segmentation fault while failsafe polls > this command line again, than gets success and tries to > do hotplug add to the sub device by uninitialized pointer > dereferencing. > This is a bug and should be fixed, thanks. > Morever, when the output is not empty but uncorrect, failsafe > returns error for its probe function while the expected > behavior is to do polling until the output is correct. > The expected behavior is for the fail-safe to return an error if the execution of the given command returns an error. The intention is that users writing such script would be able to output a blank lines in case there is nothing to probe, but still remain aware of issues during the execution of the command. The fail-safe ignores errors pertaining to absent devices due to its nature. This does not mean that it should ignore all errors and try to keep on going while everything else is on fire. The contract with the user is that "blank line" without other errors means "absent device". Garbled output or return code != 0 means runtime error and should be thrown to the user / application. > The fix changes the return value to be -ENODEV for this > sub device in the two cases. > By this way, failsafe tries to parse this sub device parameter > by exec method until the output is correct. > The issue is that this portion of the code will be heavily modified anyway. The errno handling is erroneous and must be fixed, which is in conflict with your patch. I will send the intended fix shortly, referencing this patch and the issue your highlighted, but both patch won't be compatible. > Fixes: a0194d828100 ("net/failsafe: add flexible device definition") > Fixes: 35ffe4208140 ("net/failsafe: fix missing pclose after popen") > Cc: stable@dpdk.org > > Signed-off-by: Matan Azrad > --- > drivers/net/failsafe/failsafe_args.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c > index 645c885..61c55df 100644 > --- a/drivers/net/failsafe/failsafe_args.c > +++ b/drivers/net/failsafe/failsafe_args.c > @@ -157,12 +157,16 @@ fs_execute_cmd(struct sub_device *sdev, char *cmdline) > ret = fs_parse_device(sdev, output); > if (ret) { > ERROR("Parsing device '%s' failed", output); > + ret = -ENODEV; > goto ret_pclose; > } > ret_pclose: > pclose_ret = pclose(fp); > if (pclose_ret) { > - pclose_ret = errno; > + if (errno == 0) > + errno = -(pclose_ret = ret); > + else > + pclose_ret = errno; > ERROR("pclose: %s", strerror(errno)); > errno = old_err; > return pclose_ret; > -- > 2.7.4 > Best regards, -- Gaëtan Rivet 6WIND