[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH for-4.16] efi: fix alignment of function parameters in compat mode
Roger Pau Monne writes ("[PATCH for-4.16] efi: fix alignment of function parameters in compat mode"): > Currently the max_store_size, remain_store_size and max_size in > compat_pf_efi_runtime_call are 4 byte aligned, which makes clang > complain with: > > In file included from compat.c:30: > ./runtime.c:646:13: error: passing 4-byte aligned argument to 8-byte aligned > parameter 2 of 'QueryVariableInfo' may result in an unaligned pointer access > [-Werror,-Walign-mismatch] > &op->u.query_variable_info.max_store_size, > ^ > ./runtime.c:647:13: error: passing 4-byte aligned argument to 8-byte aligned > parameter 3 of 'QueryVariableInfo' may result in an unaligned pointer access > [-Werror,-Walign-mismatch] > &op->u.query_variable_info.remain_store_size, > ^ > ./runtime.c:648:13: error: passing 4-byte aligned argument to 8-byte aligned > parameter 4 of 'QueryVariableInfo' may result in an unaligned pointer access > [-Werror,-Walign-mismatch] > &op->u.query_variable_info.max_size); > ^ > Fix this by bouncing the variables on the stack in order for them to > be 8 byte aligned. > > Note this could be done in a more selective manner to only apply to > compat code calls, but given the overhead of making an EFI call doing > an extra copy of 3 variables doesn't seem to warrant the special > casing. ... > Tagged for possible inclusion into the release, as it's a likely > candidate for backport. It shouldn't introduce any functional change > from a caller PoV. I think the risk is getting the patch wrong and not > passing the right parameters, or broken EFI implementations corrupting > data on our stack instead of doing it in xenpf_efi_runtime_call. Thanks. I have double-checked the variable names etc. Reviewed-by: Ian Jackson <iwj@xxxxxxxxxxxxxx> I agree with your analysis vis-a-vis 4.16. The current code is technically UB[1] and it seems possible that it might trigger bugs in firmware. I would like a third opinion (even though technically my review might be enough). So, subject to a review from a hypervisor maintainer: Release-Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx> Thanks, Ian. [1] Well, as far as I can tell. My copy of C99+TC1+TC2 is hopelessly unclear about unaligned pointers, and here of course we have a compiler extension too.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |