p2p on 32-bit architectures

[Sam Leffler]

On Fri, May 27, 2011 at 8:23 AM, Jouni Malinen <j at w1.fi> wrote:
> On Thu, May 26, 2011 at 04:07:05PM -0700, Sam Leffler wrote:
>> Building w/ P2P support enabled for a 32-bit target yields these complaints:
>>
>> p2p_supplicant.c: In function 'wpas_p2p_sd_request':
>> p2p_supplicant.c:1672: warning: cast to pointer from integer of different size
>> p2p_supplicant.c: In function 'wpas_p2p_sd_cancel_request':
>> p2p_supplicant.c:1702: warning: cast from pointer to integer of different size
>
> Yes... I've been too lazy to fix these two. Some of the other similar
> cases were cleaned up with better typecasts, but the cases on
> driver-based P2P management still remain.
>
>> The first issue is wpa_drv_p2p_sd_request's return value is being cast
>> to void * but the return value is u64 in driver_i.h:
>>
>> static inline u64 wpa_drv_p2p_sd_request(struct wpa_supplicant *wpa_s,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const u8 *dst,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const struct wpabuf *tlvs)
>>
>> This does not work unless we're ok blindly lopping off 32 bits.
>
> Well, this is only used if WPA_DRIVER_FLAGS_P2P_MGMT is set and the
> driver wrapper implement p2p_sd_request. No such driver wrapper exists
> in hostap.git.

Ok.  This is my first look at the p2p code.

>
>> The second complaint is the u64 cast:
>>
>> nt wpas_p2p_sd_cancel_request(struct wpa_supplicant *wpa_s, void *req)
>> {
>> ? ? ? ? if (wpa_s->drv_flags & WPA_DRIVER_FLAGS_P2P_MGMT)
>> ? ? ? ? ? ? ? ? return wpa_drv_p2p_sd_cancel_request(wpa_s, (u64) req);
>
> Similarly, this code path is not really used in hostap.git.
>
>> This needs to use uintptr_t or similar to be portable.
>
> Unfortunately, that is not really portable solution since what would be
> needed here is a maximum of sizeof(void*) and sizeof(u64). uintptr_t may
> not meet the second part of this and it could result in potential issues
> should a new driver wrapper show up with driver-based P2P service
> discovery implementation and that kernel code using 64-bit cookies to
> identify the requests.

What I meant was you need to "cast through uintptr_t" when converting
a ptr to u64.  You still need enough storage to hold the data.  I
think you're safe for a while assuming sizeof(u64) >= sizeof(void *).

>
>> This makes me believe no-one's tried this on a 32-bit arch?
>
> That particular code path has received quite limited testing because it
> is not really used at all with most drivers. I'm aware of one driver
> wrapper that is not yet in hostap.git that could potentially use this
> path. I think that it would be fine in case of 32-bit kernel, but there
> could be potential issues with 64-bit kernel and 32-bit user space.

I asked because after enabling p2p support in a ToT build of
wpa_supplicant I started seeing crashes on my 32-bit system
(kernel+user space).  Unfortunately I haven't collected a stack trace
yet.

-Sam