> 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
signature.asc
Description: Message signed with OpenPGP using GPGMail