Hi, On 07/01/18 03:28, Stefano Brivio wrote: > 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).
ops, you are right! "is not set" makes more sense.
>
>> 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 <sbri...@redhat.com>
>> Cc: Stephen Hemminger <step...@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.
>
I thought explaining "why" it could be NULL would help the casual reader
understand why we do check it.
But now that we have the check, I think it's quick to understand why we
need it.
I'd leave to whoever is going to merge the match to decide to keep or
not the comment.
>> + 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 <sbri...@redhat.com>
>
Thanks a lot, Stefano!
Should I send v2 with the commit message changed? Or can we leave this
to who will merge the change?
Regards,
--
Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
