Re: "ss -p" segfaults

[Vadim Kochan <vadim4j@gmail.com>]

On Wed, Jul 15, 2015 at 09:57:51PM +0300, Vadim Kochan wrote:
> On Wed, Jul 15, 2015 at 06:52:49PM +0000, Rustad, Mark D wrote:
> > > On Jul 15, 2015, at 9:49 AM, Rustad, Mark D <mark.d.rustad@intel.com> wrote:
> > > 
> > >> On Jul 15, 2015, at 8:12 AM, Vadim Kochan <vadim4j@gmail.com> wrote:
> > >> Would you please check this fix ?
> > >> 
> > >> diff --git a/misc/ss.c b/misc/ss.c
> > >> index 03f92fa..3a826e4 100644
> > >> --- a/misc/ss.c
> > >> +++ b/misc/ss.c
> > >> @@ -683,8 +683,8 @@ static inline void sock_addr_set_str(inet_prefix *prefix, char **ptr)
> > >> 
> > >> static inline char *sock_addr_get_str(const inet_prefix *prefix)
> > >> {
> > >> -    char *tmp ;
> > >> -    memcpy(&tmp, prefix->data, sizeof(char *));
> > >> +    char *tmp;
> > >> +    memcpy(&tmp, &prefix->data[0], sizeof(char *));
> > >>    return tmp;
> > >> }
> > > 
> > > That surely is not a fix! The destination of the memcpy is the address of an uninitialized stack variable! Both versions are equally bad.
> > 
> > I probably over-reacted, but using memcpy to access a pointer in this way is just ugly. For one thing, it circumvents any sanity-checking that the compiler can do. And changing the prefix->data to &prefix->data[0] should be exactly the same thing and therefore should not fix anything. Anyway, never mind that.
> > 
> > Looking at more of the code, it looks to me like the the string pointer in data can sometimes point to a literal string instead of allocated memory when proc is in use. Free would not be happy with that. Look at the use of variable peer in function unix_stats_print.
> > 
> Yes that right, I am already looking on this ...
> > --
> > Mark Rustad, Networking Division, Intel Corporation
> > 

I did partially revert of the buggy commit and it does not crash, but I will do
more testing, and after will send the patch and will try to prepare some
test scripts for ss.

The crash appears only if to dump processes info from /proc, which might
be caused that netlink stats returned error, probably by wrong request
(not supported attribute or flag ?).