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.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.
> 
Yes that right, I am already looking on this ...
> --
> Mark Rustad, Networking Division, Intel Corporation
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to