Re: [PATCH 2/6][next] cfg80211: Avoid clashing function prototypes

[Kees Cook <keescook@chromium.org>]

On Mon, Oct 17, 2022 at 03:34:22PM -0500, Gustavo A. R. Silva wrote:
> When built with Control Flow Integrity, function prototypes between
> caller and function declaration must match. These mismatches are visible
> at compile time with the new -Wcast-function-type-strict in Clang[1].
> 
> Fix a total of 10 warnings like these:
> 
> ../drivers/net/wireless/intersil/orinoco/wext.c:1390:27: error: incompatible function pointer types initializing 'const iw_handler' (aka 'int (*const)(struct net_device *, struct iw_request_info *, union iwreq_data *, char *)') with an expression of type 'int (struct net_device *, struct iw_request_info *, struct iw_param *, char *)' [-Wincompatible-function-pointer-types]
>         IW_HANDLER(SIOCGIWRETRY,        cfg80211_wext_giwretry),
>                                         ^~~~~~~~~~~~~~~~~~~~~~
> ../include/uapi/linux/wireless.h:357:23: note: expanded from macro 'IW_HANDLER'
>         [IW_IOCTL_IDX(id)] = func
> 
> The cfg80211 Wireless Extension handler callbacks (iw_handler) use a
> union for the data argument. Actually use the union and perform explicit
> member selection in the function body instead of having a function
> prototype mismatch. No significant binary differences were seen
> before/after changes.
> 
> Link: https://github.com/KSPP/linux/issues/234
> Link: https://reviews.llvm.org/D134831 [1]
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/net/wireless/intersil/orinoco/wext.c |  22 +--
>  include/net/cfg80211-wext.h                  |  20 +--
>  net/wireless/scan.c                          |   3 +-
>  net/wireless/wext-compat.c                   | 180 +++++++++----------
>  net/wireless/wext-compat.h                   |   8 +-
>  net/wireless/wext-sme.c                      |   5 +-
>  6 files changed, 112 insertions(+), 126 deletions(-)
> 
> diff --git a/drivers/net/wireless/intersil/orinoco/wext.c b/drivers/net/wireless/intersil/orinoco/wext.c
> index b8eb5d60192f..dea1ff044342 100644
> --- a/drivers/net/wireless/intersil/orinoco/wext.c
> +++ b/drivers/net/wireless/intersil/orinoco/wext.c
> @@ -1363,31 +1363,31 @@ static const struct iw_priv_args orinoco_privtab[] = {
>  
>  static const iw_handler	orinoco_handler[] = {
>  	IW_HANDLER(SIOCSIWCOMMIT,	orinoco_ioctl_commit),
> -	IW_HANDLER(SIOCGIWNAME,		(iw_handler)cfg80211_wext_giwname),
> +	IW_HANDLER(SIOCGIWNAME,		cfg80211_wext_giwname),

This hunk should be in the orinoco patch, I think?


> [...]
> +	[IW_IOCTL_IDX(SIOCGIWRETRY)]    = cfg80211_wext_giwretry,

The common practice seems to be to use IW_HANDLER instead of open-coding
it like this.

	IW_HANDLER(SIOCGIWRETRY,	cfg80211_wext_giwretry),

-- 
Kees Cook