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

--
Mark Rustad, Networking Division, Intel Corporation

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

Reply via email to