On Fri, 21 Mar 2025 13:40:57 -0700
Kees Cook <[email protected]> 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 <[email protected]>
> ---
> 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;
> }
>
