From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Brivio Subject: Re: [iproute2 2/2] ss: fix NULL pointer access when parsing unix sockets with oldformat Date: Sat, 6 Jan 2018 20:28:18 +0100 Message-ID: <20180106202818.2160a475@elisabeth> References: <20180106183150.10530-1-a@unstable.cc> <20180106183150.10530-2-a@unstable.cc> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, Stephen Hemminger , Phil Sutter To: Antonio Quartulli Return-path: Received: from mx1.redhat.com ([209.132.183.28]:41112 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752108AbeAFT2Z (ORCPT ); Sat, 6 Jan 2018 14:28:25 -0500 In-Reply-To: <20180106183150.10530-2-a@unstable.cc> Sender: netdev-owner@vger.kernel.org List-ID: On Sun, 7 Jan 2018 02:31:50 +0800 Antonio Quartulli wrote: > When parsing and printing the unix sockets in unix_show(), > if the oldformat is detected, the peer_name member of the sockstat > object is left uninitialized (NULL). Luckily, it is initialized. I'd rather say: [...] object is not set (NULL). > For this reason, if a filter has been specified on the command line, > a strcmp() will crash when trying to access it. > > Avoid crash by checking that peer_name is not NULL before > passing it to strcmp(). > > Cc: Stefano Brivio > Cc: Stephen Hemminger > Signed-off-by: Antonio Quartulli Fixes: 2d0e538f3e1c ("ss: Drop list traversal from unix_stats_print()") > [...] > > diff --git a/misc/ss.c b/misc/ss.c > index b35859dc..29a25070 100644 > --- a/misc/ss.c > +++ b/misc/ss.c > @@ -3711,7 +3711,10 @@ static int unix_show(struct filter *f) > }; > > memcpy(st.local.data, &u->name, sizeof(u->name)); > - if (strcmp(u->peer_name, "*")) > + /* when parsing the old format rport is set to 0 and > + * therefore peer_name remains NULL > + */ Maybe this comment is a bit redundant, but I don't have a strong preference either. > + if (u->peer_name && strcmp(u->peer_name, "*")) > memcpy(st.remote.data, &u->peer_name, > sizeof(u->peer_name)); > if (run_ssfilter(f->f, &st) == 0) { FWIW: Reviewed-by: Stefano Brivio -- Stefano