LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] lkdtm/heap: Avoid __alloc_size hint warning
@ 2021-08-18  4:45 Kees Cook
  2021-08-18 14:01 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2021-08-18  4:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Kees Cook, linux-kernel, linux-hardening

Once __alloc_size hints have been added, the compiler will
(correctly!) see this as an overflow. We are, however, trying to test
for this condition, so work around it with a volatile int.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/misc/lkdtm/heap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
index 3d9aae5821a0..e59fcbe00ae0 100644
--- a/drivers/misc/lkdtm/heap.c
+++ b/drivers/misc/lkdtm/heap.c
@@ -12,6 +12,8 @@ static struct kmem_cache *double_free_cache;
 static struct kmem_cache *a_cache;
 static struct kmem_cache *b_cache;
 
+static volatile int __offset = 1;
+
 /*
  * If there aren't guard pages, it's likely that a consecutive allocation will
  * let us overflow into the second allocation without overwriting something real.
@@ -24,7 +26,7 @@ void lkdtm_VMALLOC_LINEAR_OVERFLOW(void)
 	two = vzalloc(PAGE_SIZE);
 
 	pr_info("Attempting vmalloc linear overflow ...\n");
-	memset(one, 0xAA, PAGE_SIZE + 1);
+	memset(one, 0xAA, PAGE_SIZE + __offset);
 
 	vfree(two);
 	vfree(one);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] lkdtm/heap: Avoid __alloc_size hint warning
  2021-08-18  4:45 [PATCH] lkdtm/heap: Avoid __alloc_size hint warning Kees Cook
@ 2021-08-18 14:01 ` Greg Kroah-Hartman
  2021-08-18 17:32   ` Kees Cook
  0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2021-08-18 14:01 UTC (permalink / raw)
  To: Kees Cook; +Cc: linux-kernel, linux-hardening

On Tue, Aug 17, 2021 at 09:45:40PM -0700, Kees Cook wrote:
> Once __alloc_size hints have been added, the compiler will
> (correctly!) see this as an overflow. We are, however, trying to test
> for this condition, so work around it with a volatile int.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  drivers/misc/lkdtm/heap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
> index 3d9aae5821a0..e59fcbe00ae0 100644
> --- a/drivers/misc/lkdtm/heap.c
> +++ b/drivers/misc/lkdtm/heap.c
> @@ -12,6 +12,8 @@ static struct kmem_cache *double_free_cache;
>  static struct kmem_cache *a_cache;
>  static struct kmem_cache *b_cache;
>  
> +static volatile int __offset = 1;

Perhaps a comment here as to why volatile is ok to use?  That feels like
it is a hack around the compiler of today, what happens tomorrow when
newer versions decide to ignore volatile as it "knows" no one ever
changes it?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] lkdtm/heap: Avoid __alloc_size hint warning
  2021-08-18 14:01 ` Greg Kroah-Hartman
@ 2021-08-18 17:32   ` Kees Cook
  0 siblings, 0 replies; 3+ messages in thread
From: Kees Cook @ 2021-08-18 17:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, linux-hardening

On Wed, Aug 18, 2021 at 04:01:26PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Aug 17, 2021 at 09:45:40PM -0700, Kees Cook wrote:
> > Once __alloc_size hints have been added, the compiler will
> > (correctly!) see this as an overflow. We are, however, trying to test
> > for this condition, so work around it with a volatile int.
> > 
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  drivers/misc/lkdtm/heap.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
> > index 3d9aae5821a0..e59fcbe00ae0 100644
> > --- a/drivers/misc/lkdtm/heap.c
> > +++ b/drivers/misc/lkdtm/heap.c
> > @@ -12,6 +12,8 @@ static struct kmem_cache *double_free_cache;
> >  static struct kmem_cache *a_cache;
> >  static struct kmem_cache *b_cache;
> >  
> > +static volatile int __offset = 1;
> 
> Perhaps a comment here as to why volatile is ok to use?  That feels like
> it is a hack around the compiler of today, what happens tomorrow when
> newer versions decide to ignore volatile as it "knows" no one ever
> changes it?

Sure, I can do that. LKDTM uses this a lot because it, by definition,
means the compiler cannot assume it knows anything about its value. (And
as such reloads from memory at every use, which is why it's frowned upon
anywhere else in the kernel.)

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-18 17:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18  4:45 [PATCH] lkdtm/heap: Avoid __alloc_size hint warning Kees Cook
2021-08-18 14:01 ` Greg Kroah-Hartman
2021-08-18 17:32   ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).