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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Oct 2022 19:41:48 -0700
From:   Kees Cook <keescook@...omium.org>
To:     "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc:     Johannes Berg <johannes@...solutions.net>,
        Kalle Valo <kvalo@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-hardening@...r.kernel.org
Subject: Re: [PATCH 2/6][next] cfg80211: Avoid clashing function prototypes

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@...nel.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