Re: A bug in message_add_payload()

Hello Hakan,

Please give me time to do more investigation so that I can give you more accurate information.

Regards,
Aref Taidi

-----Original Message-----
From: Hakan Olsson [mailto:ho@crt.se]
Sent: 16 April 2003 13:11
To: Aref Taidi
Cc: 'bugs@openbsd.org'; markus@openbsd.org Subject: RE: A bug in message_add_payload()

On Wed, 16 Apr 2003, Aref Taidi wrote:

> The problem I had was that message_check_duplicate() called message_free()

This 'payload' data was allocated by the malloc() call prior to the realloc() in message_add_payload(), or in the message_index_payload() function. As far as I recall, and after a quick search through the source, there are no other places where 'struct payload' objects are allocated.

The realloc() only concerned the msg->iov array, which was used and freed a few lines above these lines in message_free(), without errors.

Do you need help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related openbsd.org Links advocacy announce bugs ipv6 misc ports ports-changes security-announce smp source-changes tech Request more information about Pantek

If you get an error here, I suspect the TAILQ handling is faulty somehow. Or we may need to track where memory used in that TAILQ is allocated and freed. AFAICT this looks sane, but perhaps I've missed something here.

>
> I then noticed that memory obtained for msg->iov originally in

How did you fix this? Show me code, please.

As struct iov is defined as (/usr/includes/sys/uio.h)

struct iovec {

        void    *iov_base;      /* Base address. */
        size_t   iov_len;       /* Length. */

and the realloc's new size here is (iov_len + 1) * sizeof (struct iovec), and both variables of the the newly allocated iovec struct's (i.e. iov_base and iov_len) are filled in (assuming the realloc does not fail), I do not see how clearing those bytes prior to this can possibly have any effect.

In fact, as far as I understand compiler optimizations (not a lot, admittedly), this zeroing would be a good candidate for an optimizer to remove from the generated code.

>From message_add_payload() :

Do you need more help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related openbsd.org Links advocacy announce bugs ipv6 misc ports ports-changes security-announce smp source-changes tech Request more information about Pantek

  new_iov

• (struct iovec *)realloc (msg->iov, (msg->iovlen + 1) * sizeof *msg->iov); if (!new_iov) { log_error ("message_add_payload: realloc (%p, %lu) failed", msg->iov, (msg->iovlen + 1) * (unsigned long)sizeof *msg->iov); free (payload_node); return -1; } msg->iov = new_iov; new_iov[msg->iovlen].iov_base = buf; new_iov[msg->iovlen].iov_len = sz; msg->iovlen++;

You're proposing to add something like

  memset (&new_iov[msg->iovlen], 0, sizeof *msg->iov);

right next to the "msg->iov = new_iov;" line ?

I'm sorry, I really do not follow you here.

/H

--
Håkan Olsson         (+46) 708 437 337     Carlstedt Research
Unix, Networking, Security      (+46) 31 701 4264        & Technology AB