Re: [PATCH 1/5] treewide: Replace kfree() casts with union members

[David Laight <david.laight.linux@gmail.com>]

On Fri, 21 Mar 2025 13:40:57 -0700
Kees Cook <kees@kernel.org> wrote:

> As a prerequisite to being able to optionally take the address of any
> lvalues used in a call to kfree(), replace casts to the kfree() argument
> with unions to include an actual pointer.

Having different names for the argument to kfree() and other uses
of the data is just brain-dead and will make reading code harder
and writing code that access data after it is freed so much easier.

	David

> 
> This is an example subset. There are another handful remaining:
> 
> $ git grep '\bkfree((void \*)'
> arch/mips/alchemy/common/dbdma.c:       kfree((void *)ctp->cdb_membase);
> arch/s390/kernel/crash_dump.c:  kfree((void *)(unsigned long)addr);
> drivers/crypto/atmel-sha204a.c: kfree((void *)i2c_priv->hwrng.priv);
> drivers/infiniband/hw/cxgb4/mem.c:              kfree((void *) (unsigned long) mhp->kva);
> drivers/isdn/mISDN/fsm.c:       kfree((void *) fsm->jumpmatrix);
> drivers/misc/altera-stapl/altera.c:           kfree((void *)vars[variable_id]);
> drivers/misc/altera-stapl/altera.c:                             kfree((void *)vars[i]);
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h:                        kfree((void *)x); \
> drivers/net/ethernet/qlogic/qed/qed_main.c:     kfree((void *)cdev);
> drivers/net/usb/cx82310_eth.c:  kfree((void *)dev->partial_data);
> drivers/net/usb/cx82310_eth.c:  kfree((void *)dev->partial_data);
> drivers/scsi/snic/snic_io.c:            kfree((void *)rqi->sge_va);
> drivers/scsi/snic/snic_io.c:                    kfree((void *)rqi->sge_va);
> drivers/staging/rtl8723bs/os_dep/os_intfs.c:    /* kfree((void *)padapter); */
> drivers/video/fbdev/grvga.c:            kfree((void *)virtual_start);
> drivers/video/fbdev/grvga.c:                    kfree((void *)info->screen_base);
> drivers/xen/grant-table.c:                      kfree((void *)page_private(pages[i]));
> net/ieee802154/nl802154.c:      kfree((void *)cb->args[0]);
> net/sched/em_ipset.c:           kfree((void *) em->data);
> net/sched/em_meta.c:    kfree((void *) v->val);
> 
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>  include/linux/netlink.h | 1 +
>  include/net/pkt_cls.h   | 5 ++++-
>  net/sched/ematch.c      | 2 +-
>  net/wireless/nl80211.c  | 2 +-
>  4 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> index c3ae84a77e16..26eb9eea8a74 100644
> --- a/include/linux/netlink.h
> +++ b/include/linux/netlink.h
> @@ -295,6 +295,7 @@ struct netlink_callback {
>  	bool			strict_check;
>  	union {
>  		u8		ctx[NETLINK_CTX_SIZE];
> +		void *		ptr;
>  
>  		/* args is deprecated. Cast a struct over ctx instead
>  		 * for proper type safety.
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index c64fd896b1f9..4faf8d6eed1d 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -403,7 +403,10 @@ struct tcf_ematch_ops;
>   */
>  struct tcf_ematch {
>  	struct tcf_ematch_ops * ops;
> -	unsigned long		data;
> +	union {
> +		unsigned long	data;
> +		void *		ptr;
> +	};
>  	unsigned int		datalen;
>  	u16			matchid;
>  	u16			flags;
> diff --git a/net/sched/ematch.c b/net/sched/ematch.c
> index 5c1235e6076a..f4b00e7aca6a 100644
> --- a/net/sched/ematch.c
> +++ b/net/sched/ematch.c
> @@ -411,7 +411,7 @@ void tcf_em_tree_destroy(struct tcf_ematch_tree *tree)
>  			if (em->ops->destroy)
>  				em->ops->destroy(em);
>  			else if (!tcf_em_is_simple(em))
> -				kfree((void *) em->data);
> +				kfree(em->ptr);
>  			module_put(em->ops->owner);
>  		}
>  	}
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index d7d3da0f6833..b5a3ae07d84c 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -3270,7 +3270,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
>  
>  static int nl80211_dump_wiphy_done(struct netlink_callback *cb)
>  {
> -	kfree((void *)cb->args[0]);
> +	kfree(cb->ptr);
>  	return 0;
>  }
>