netdev - Re: "ss -p" segfaults

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <C873AEFC-4840-4651-8AE3-F9D21DDDD0DA@intel.com>
Date:	Wed, 15 Jul 2015 18:52:49 +0000
From:	"Rustad, Mark D" <mark.d.rustad@...el.com>
To:	Vadim Kochan <vadim4j@...il.com>
CC:	Marc Dietrich <marvin24@....de>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: "ss -p" segfaults

> On Jul 15, 2015, at 9:49 AM, Rustad, Mark D <mark.d.rustad@...el.com> wrote:
> 
>> On Jul 15, 2015, at 8:12 AM, Vadim Kochan <vadim4j@...il.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.

--
Mark Rustad, Networking Division, Intel Corporation

Download attachment "signature.asc" of type "application/pgp-signature" (842 bytes)