dhcpcd-discuss

On 20/12/2019 16:08, Graham Breed wrote:
> I upgraded dhcpcd and it consistently failed to start up.  I tracked the
> problem down to a '\0' being added beyond the allocated memory.  There's
> a simple patch:
>
> > ---
> >   src/if-options.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/if-options.c b/src/if-options.c
> > index f3f12316..6bf4ad04 100644
> > --- a/src/if-options.c
> > +++ b/src/if-options.c
> > @@ -720,7 +720,7 @@ parse_option(struct dhcpcd_ctx *ctx, const char *ifname, struct if_options *ifo,
> >   			break;
> >   		}
> >   		dl = (size_t)s;
> > -		if (s == -1 || (ifo->script = malloc(dl)) == NULL) {
> > +		if (s == -1 || (ifo->script = malloc(dl + 1)) == NULL) {
> >   			ifo->script = NULL;
> >   			logerr(__func__);
> >   			return -1;
> > --
>
>
> And, if it isn't that simple, at least this works for me.  This is the
> valgrind warning that pointed me to it:
>
> > ==8231== Invalid write of size 1
> > ==8231==    at 0x40D674: parse_str (if-options.c:397)
> > ==8231==    by 0x40E03E: parse_option (if-options.c:728)
> > ==8231==    by 0x412F0A: parse_config_line (if-options.c:2205)
> > ==8231==    by 0x413C97: read_config (if-options.c:2559)
> > ==8231==    by 0x40823F: main (dhcpcd.c:1722)
> > ==8231==  Address 0x5237758 is 0 bytes after a block of size 24 alloc'd
> > ==8231==    at 0x4C2DF06: malloc (vg_replace_malloc.c:309)
> > ==8231==    by 0x40DFC6: parse_option (if-options.c:723)
> > ==8231==    by 0x412F0A: parse_config_line (if-options.c:2205)
> > ==8231==    by 0x413C97: read_config (if-options.c:2559)
> > ==8231==    by 0x40823F: main (dhcpcd.c:1722)
>
> This is dhcpcd 8.1.3 on Linux.  Versions back to 7.2.0 have the same
> problem.

Yoiks!

Thanks for the patch - I elected to solve it a different way.
Because we only care about the NULL value when passing PARSE_STRING_NULL as a 
flag, I elected to increase the storage requirement of the return - and check if 
the passed in length has space to actually write it as well.

This makes it easier to use in other places. Not that it is currently, but you 
never know.

Fixed here in dhcpcd-7 (also pused to dhcpcd-8 and master):
https://roy.marples.name/cgit/dhcpcd.git/commit/?h=dhcpcd-7&id=dd162e7c95819587ca49c59832eb31059e774233

Thanks

Roy