Re: [PATCH 1/3] connect: treat generic proxy processes like ssh processes

From: Johannes Sixt <j6t@kdbg.org>
Date: May 17, 2011, 20:14 UTC
Message-ID: <4DD2D733.3070004@kdbg.org>
In-Reply-To: <20110517055422.GD10048@sigill.intra.peff.net>

Am 17.05.2011 07:54, schrieb Jeff King:

> On Mon, May 16, 2011 at 09:57:58PM +0200, Johannes Sixt wrote:
>> In my implementation, I xmalloced the pointer array and leaked it.

I noticed that it actually isn't leaked because finish_connect() frees it. For this reason, I actually have to wonder why your version that stored a pointer to automatic storage in ->argv worked.

> We also need to worry about the contents of each argv[] element, no? So
> we should be xstrdup()ing the host and port, which point into some
> string which gets passed to us. I didn't trace its provenance but I
> think it is better to be defensive.

I would not worry too much today. Of course, functions other than start_command() might begin to access ->argv[i] with i > 0 later, but then we have to audit all users of struct child_process anyway. Currently, only start_command() uses these values, which is always called at a time when they are still valid.

-- Hannes