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

From: Kees Cook
Date: Mon Oct 17 2022 - 22:41:58 EST


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@xxxxxxxxxx>
> ---
> 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