[PATCH 1/2] hostapd: make it possible to remove addresses from maclists

[Jouni Malinen]

On Tue, Feb 18, 2014 at 11:36:34AM +0100, Emanuel Taube wrote:
> It is already possible to add mac addresses at runtime.
> This patch allows also to remove some of them by the prefix "-"
> in the address file.

> diff --git a/hostapd/config_file.c b/hostapd/config_file.c
> @@ -143,14 +145,31 @@ static int hostapd_config_read_maclist(const char *fname,
> +		if (remove) {
> +			for (i = 0; i < *num; i++)
> +				if (os_memcmp((*acl)[i].addr, addr, ETH_ALEN) ==
> +				    0) {
> +					*acl = os_remove_in_array(
> +						*acl, *num,
> +						sizeof(**acl), i);

This looks like it could leak memory on failure (and even worse, crash
on NULL dereference if the list were to have more entries).. Not really
expected to happen in practice, but whenever I see realloc and the
pointer be reassigned without being verified, this triggers warnings..

In addition, it looks like this loop would skip checking the entry
following the one that got removed. Not that there is an expectation for
there to be multiple entries for the same MAC address, but since that
loop did not break out on the first match, it looks like it is trying to
handle that case as well.

> diff --git a/src/utils/os.h b/src/utils/os.h
> @@ -549,6 +549,25 @@ static inline void * os_realloc_array(void *ptr, size_t nmemb, size_t size)
>  	return os_realloc(ptr, nmemb * size);
> +static inline void * os_remove_in_array(void *ptr, size_t nmemb, size_t size,
> +				        unsigned int idx)
> +{
> +	if (idx >= nmemb)
> +		return ptr;
> +	if (idx < nmemb - 1)
> +		os_memmove(ptr + idx * size, ptr + (idx + 1) * size,
> +			   (nmemb - idx - 1) * size);
> +
> +	return os_realloc_array(ptr, nmemb-1, size);
> +}

Is it really worth the effort to realloc the buffer here? I would
consider changing this to a void function with the last step removed.

-- 
Jouni Malinen                                            PGP id EFC895FA