On Thu, Nov 28, 2024 at 09:08:34AM +0100, Lorenzo Bianconi wrote:
> Introduce Automated Frequency Coordination (AFC) support for UNII-5 and
> UNII-7 6GHz bands.
> AFC client will connect to AFCD providing AP related parameter for AFC
> coordinator (e.g. geolocation, supported frequencies, ..).
> AFC is required for Standard Power Devices (SPDs) to determine a lists
> of channels and EIRP/PSD powers that are available in the 6GHz spectrum.
> AFC hostapd client is tested with AFC DUT Test Harness [0].
> diff --git a/hostapd/Makefile b/hostapd/Makefile
> +ifdef CONFIG_IEEE80211AX
> +ifdef CONFIG_AFC
> +CFLAGS += -DCONFIG_AFC
> +OBJS += ../src/ap/afc.o
> +LIBS += -ljson-c
> +endif
> +endif
Did you consider using the internal JSON implementation that DPP is
using in hostapd builds? It would be nice to be able to avoid
unnecessary extra dependencies or at least to clearly justify need for
such in the commit message.
> diff --git a/hostapd/config_file.c b/hostapd/config_file.c
> +#ifdef CONFIG_AFC
> +static int hostapd_afc_parse_cert_ids(struct hostapd_config *conf, char *pos)
> +{
> + c = os_realloc_array(c, count + 1, sizeof(*c));
> + if (!c)
> + return -ENOMEM;
That would leak memory on failure, i.e., all realloc cases need to be
able to free the previously allocated memory if NULL is returned. In
this case, this would actually leak all the allocations in earlier
struct cert_id instances as well as the array of these.
> + c[i].rulset = os_malloc(os_strlen(pos) + 1);
> + if (!c[i].rulset)
> + goto error;
> +
> + os_strlcpy(c[i].rulset, pos, os_strlen(pos) + 1);
os_strdup() should be used instead of malloc+strlcpy.
> +#ifdef CONFIG_AFC
> + } else if (os_strcmp(buf, "afcd_sock") == 0) {
> + conf->afc.socket = os_malloc(os_strlen(pos) + 1);
> + if (!conf->afc.socket)
> + return 1;
> +
> + os_strlcpy(conf->afc.socket, pos, os_strlen(pos) + 1);
More examples of where os_strldup() should be used. In addition, these
should take care of the possibility of the same parameter being set and
updated, i.e., the old conf->afc.socket value needs to be freed before
overriding the pointer.
> diff --git a/src/ap/afc.c b/src/ap/afc.c
> @@ -0,0 +1,1109 @@
> +#include <json-c/json.h>
> +#include <sys/un.h>
> +#include <time.h>
> +
> +#include "utils/includes.h"
By convention, utils/includes.h should be included before additional
system header files.
> +static struct json_object *
> +hostapd_afc_build_location_request(struct hostapd_iface *iface)
...
I did not review the actual implementation here yet. It would be much
more convenient to be able to experiment with this using an hwsim test
case as noted in an earlier email.
--
Jouni Malinen PGP id EFC895FA
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
