[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <201906270908.28E5E1FDC3@keescook>
Date: Thu, 27 Jun 2019 09:21:42 -0700
From: Kees Cook <keescook@...omium.org>
To: Vegard Nossum <vegard.nossum@...il.com>
Cc: "Gote, Nitin R" <nitin.r.gote@...el.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: Regarding have kfree() (and related) set the pointer to NULL too
On Thu, Jun 27, 2019 at 01:45:06PM +0200, Vegard Nossum wrote:
> On Thu, 27 Jun 2019 at 12:23, Gote, Nitin R <nitin.r.gote@...el.com> wrote:
> > Hi,
> >
> > I’m looking into “have kfree() (and related) set the pointer to NULL too” task.
> >
> > As per my understanding, I did below changes :
> >
> > Could you please provide some points on below ways ?
> > @@ -3754,6 +3754,7 @@ void kfree(const void *objp)
> > debug_check_no_obj_freed(objp, c->object_size);
> > __cache_free(c, (void *)objp, _RET_IP_);
> > local_irq_restore(flags);
> > + objp = NULL;
> >
> > }
>
> This will not do anything, since the assignment happens to the local
> variable inside kfree() rather than to the original expression that
> was passed to it as an argument.
>
> Consider that the code in the caller looks like this:
>
> void *x = kmalloc(...);
> kfree(x);
> pr_info("x = %p\n", x);
>
> this will still print "x = (some non-NULL address)" because the
> variable 'x' in the caller still retains its original value.
>
> You could try wrapping kfree() in a C macro, something like
>
> #define kfree(x) real_kfree(x); (x) = NULL;
Right, though we want to avoid silent double-evaluation, so we have to
do some macro tricks. I suspect the starting point is something like:
#define kfree(x) \
do { \
typeof(x) *ptr = &(x); \
real_kfree(*ptr); \
*ptr = NULL; \
} while (0)
However, there are a non-zero number of places in the kernel where kfree()
is used on things that are not simple memory references, like function
return values, or copies of the actual reference:
kfree(get_my_allocation(foo));
or
previous = something->allocation;
...
kfree(prevous)
So the larger work is figuring out how to gracefully deal with those
using a reasonable API, or through refactoring.
However, before getting too far, it's worth going though past
use-after-free vulnerabilities to figure out how many would have been
rendered harmless (NULL deref instead of UaF) with this change. Has this
been studied before, etc. With this information it's easier to decide
if the benefit of this refactoring is worth the work to do it.
--
Kees Cook
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
