> > - if (type & MAC_ADDR_RAND_SCHED_SCAN) {
> > - if (wpas_mac_addr_rand_scan_set(wpa_s, MAC_ADDR_RAND_SCHED_SCAN,
> > - addr, mask))
> > - return -1;
>
> The changes were to reintroduce some of the return -1 statements that
> had been removed in your patch without any clear explanation of why they
> were removed. If such changes are needed, please provide a separate
> patch with those changes and a commit log that explains the need.
Thanks for catching this. I think it was just something I missed while
rebasing these patches from a much older version of wpa_supplicant.
> This would result in memory leaks since that allocated heap memory is
> not freed anywhere. I'd assume this part is related to that "weird
> pointer ownership". It should really be handled in a separate patch and
> with clear explanation of why it is needed and changes to clearly
> describe how this new allocated buffer can be safely freed.
>
> I'm attaching the patch for the remaining changes that I did not yet
> apply due to that memory leak and hope of getting it split into two
> patches. This has some coding style cleanup compared to the version you
> sent to get rid of a compiler warnings and to be a bit more consistent
> with the coding style used in hostap.git.
The weird pointer ownership comes from the fact that the MAC address
randomization byte arrays could be simultaneously referenced by the
scan params and the wpa_supplicant struct itself, leading to
situations where we might free it in one place and leave it dangling
in the other struct. For example, a wpa_scan_free_params invalidates
the pointer held in the relevant wpa_supplicant struct. I chose to
allocate a new array and copy to avoid this because I think I was
seeing crashes at the time I originally wrote this code for that
reason.
Either way, the fix for this is pretty simple. Thanks for the review.
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
