[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 6 Jul 2019 20:52:44 +0000
From: "Gote, Nitin R" <nitin.r.gote@...el.com>
To: Kees Cook <keescook@...omium.org>, Vegard Nossum <vegard.nossum@...il.com>
CC: "kernel-hardening@...ts.openwall.com"
<kernel-hardening@...ts.openwall.com>
Subject: RE: Regarding have kfree() (and related) set the pointer to NULL too
> -----Original Message-----
> From: Kees Cook [mailto:keescook@...omium.org]
> Sent: Thursday, June 27, 2019 9:52 PM
> To: Vegard Nossum <vegard.nossum@...il.com>
> Cc: Gote, Nitin R <nitin.r.gote@...el.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.
>
As per my understanding from above comment is that,
First need to check below changes are harmless or not using tool like KASAN (KASAN report).
> #define kfree(x) \
> do { \
> typeof(x) *ptr = &(x); \
> real_kfree(*ptr); \
> *ptr = NULL; \
> } while (0)
If I misunderstood this, Could you please elaborate this? As I'm not able to conclude on this completely.
--
Nitin Gote.
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.
