[RFC 0/5] slab: Set freed variables to NULL by default

Kees Cook posted 5 patches 10 months, 3 weeks ago
arch/mips/alchemy/common/dbdma.c |  2 +-
include/linux/compiler_types.h   | 10 ++++++++++
include/linux/netlink.h          |  1 +
include/linux/slab.h             | 33 ++++++++++++++++++++++++++++++--
include/net/pkt_cls.h            |  5 ++++-
io_uring/futex.c                 |  2 +-
io_uring/io_uring.c              | 12 ++++++------
kernel/bpf/core.c                |  3 ++-
mm/slab_common.c                 | 12 ++++++++----
mm/slub.c                        |  6 +++---
net/sched/ematch.c               |  2 +-
net/wireless/nl80211.c           |  2 +-
12 files changed, 69 insertions(+), 21 deletions(-)
[RFC 0/5] slab: Set freed variables to NULL by default
Posted by Kees Cook 10 months, 3 weeks ago
Hi!

This is very much an RFC series, but I wanted to make sure it actually
worked before I proposed it. This series seeks to give kfree() the
side-effect of assigning NULL to the kfree() argument when possible.
This would make a subset of "dangling pointer" flaws turn into NULL
derefs instead of Use-After-Free[1]. It effectively turns:

	kfree(var);

into:

	kfree(var);
	var = NULL;

when "var" is actually addressable. (i.e. not "kfree(get_ptrs())" etc.)

It depends on a builtin, __builtin_is_lvalue(), which is not landed in any
compiler yet, but I do have it working in a Clang patch[2]. This should
be essentially free (pardon the pun), so I think if folks can tolerate
a little bit of renaming needed for when kfree is needed as a function
and not a macro, it should be good. Please let me know what you think.

Thanks!

-Kees

(Yes, I'm still working on the kmalloc_objs() series, but I needed to
take a break from fixing all the allocation corner cases I've found there.)

[1] https://github.com/KSPP/linux/issues/87
[2] https://github.com/kees/llvm-project/commits/builtin_is_lvalue/

Kees Cook (5):
  treewide: Replace kfree() casts with union members
  treewide: Prepare for kfree() to __kfree() rename
  compiler_types: Introduce __is_lvalue()
  slab: Set freed variables to NULL by default
  [DEBUG] slab: Report number of NULLings

 arch/mips/alchemy/common/dbdma.c |  2 +-
 include/linux/compiler_types.h   | 10 ++++++++++
 include/linux/netlink.h          |  1 +
 include/linux/slab.h             | 33 ++++++++++++++++++++++++++++++--
 include/net/pkt_cls.h            |  5 ++++-
 io_uring/futex.c                 |  2 +-
 io_uring/io_uring.c              | 12 ++++++------
 kernel/bpf/core.c                |  3 ++-
 mm/slab_common.c                 | 12 ++++++++----
 mm/slub.c                        |  6 +++---
 net/sched/ematch.c               |  2 +-
 net/wireless/nl80211.c           |  2 +-
 12 files changed, 69 insertions(+), 21 deletions(-)

-- 
2.34.1
Re: [RFC 0/5] slab: Set freed variables to NULL by default
Posted by Harry Yoo 10 months, 2 weeks ago
On Fri, Mar 21, 2025 at 01:40:56PM -0700, Kees Cook wrote:
> Hi!
> 
> This is very much an RFC series, but I wanted to make sure it actually
> worked before I proposed it. This series seeks to give kfree() the
>` side-effect of assigning NULL to the kfree() argument when possible.
> This would make a subset of "dangling pointer" flaws turn into NULL
> derefs instead of Use-After-Free[1]. It effectively turns:
> 
> 	kfree(var);
> 
> into:
> 
> 	kfree(var);
> 	var = NULL;
> 
> when "var" is actually addressable. (i.e. not "kfree(get_ptrs())" etc.)

Maybe a dumb question, but if 'var' is a local variable, is it common that
a dangling pointer in a local variable to lead to a use-after-free bug?

Also, I don't expect this to have a significant performance impact, but have
you measured it and is the overhead low enough to enable it unconditionally?

> It depends on a builtin, __builtin_is_lvalue(), which is not landed in any
> compiler yet, but I do have it working in a Clang patch[2]. This should
> be essentially free (pardon the pun), so I think if folks can tolerate
> a little bit of renaming needed for when kfree is needed as a function
> and not a macro, it should be good. Please let me know what you think.

I don't have a strong opinion on the slab side, but now users of kfree
should be aware of this subtle change (e.g., when debugging code).

-- 
Cheers,
Harry (formerly known as Hyeonggon)


> Thanks!
> 
> -Kees
> 
> (Yes, I'm still working on the kmalloc_objs() series, but I needed to
> take a break from fixing all the allocation corner cases I've found there.)
> 
> [1] https://github.com/KSPP/linux/issues/87
> [2] https://github.com/kees/llvm-project/commits/builtin_is_lvalue/
> 
> Kees Cook (5):
>   treewide: Replace kfree() casts with union members
>   treewide: Prepare for kfree() to __kfree() rename
>   compiler_types: Introduce __is_lvalue()
>   slab: Set freed variables to NULL by default
>   [DEBUG] slab: Report number of NULLings
> 
>  arch/mips/alchemy/common/dbdma.c |  2 +-
>  include/linux/compiler_types.h   | 10 ++++++++++
>  include/linux/netlink.h          |  1 +
>  include/linux/slab.h             | 33 ++++++++++++++++++++++++++++++--
>  include/net/pkt_cls.h            |  5 ++++-
>  io_uring/futex.c                 |  2 +-
>  io_uring/io_uring.c              | 12 ++++++------
>  kernel/bpf/core.c                |  3 ++-
>  mm/slab_common.c                 | 12 ++++++++----
>  mm/slub.c                        |  6 +++---
>  net/sched/ematch.c               |  2 +-
>  net/wireless/nl80211.c           |  2 +-
>  12 files changed, 69 insertions(+), 21 deletions(-)
> 
> -- 
> 2.34.1
>