Re: [iproute2 2/2] ss: fix NULL pointer access when parsing unix sockets with oldformat

[Stefano Brivio <sbrivio@redhat.com>]

On Sun,  7 Jan 2018 02:31:50 +0800
Antonio Quartulli <a@unstable.cc> 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 <sbrivio@redhat.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Antonio Quartulli <a@unstable.cc>

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 <sbrivio@redhat.com>

-- 
Stefano