lkml.org 
[lkml]   [2019]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] asm/sections: Check for overflow in memory_contains()
On Tue, Dec 17, 2019 at 11:28:31AM +0100, Russell King - ARM Linux admin wrote:
> On Tue, Dec 17, 2019 at 11:22:38AM +0100, Vincent Whitchurch wrote:
> > ARM uses memory_contains() from its stacktrace code via this function:
> >
> > static inline bool in_entry_text(unsigned long addr)
> > {
> > return memory_contains(__entry_text_start, __entry_text_end,
> > (void *)addr, 1);
> > }
> >
> > addr is taken from the stack and can be a completely invalid. If addr
> > is 0xffffffff, there is an overflow in the pointer arithmetic in
> > memory_contains() and in_entry_text() incorrectly returns true.
> >
> > Fix this by adding an overflow check. The check is done on unsigned
> > longs to avoid undefined behaviour.
> >
> > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> > ---
> > include/asm-generic/sections.h | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> > index d1779d442aa5..e6e1b381c5df 100644
> > --- a/include/asm-generic/sections.h
> > +++ b/include/asm-generic/sections.h
> > @@ -105,7 +105,15 @@ static inline int arch_is_kernel_initmem_freed(unsigned long addr)
> > static inline bool memory_contains(void *begin, void *end, void *virt,
> > size_t size)
> > {
> > - return virt >= begin && virt + size <= end;
> > + unsigned long membegin = (unsigned long)begin;
> > + unsigned long memend = (unsigned long)end;
> > + unsigned long objbegin = (unsigned long)virt;
> > + unsigned long objend = objbegin + size;
> > +
> > + if (objend < objbegin)
> > + return false;
> > +
> > + return objbegin >= membegin && objend <= memend;
>
> Would merely changing to:
>
> return virt >= begin && virt <= end - size;
>
> be sufficient ? Is end - size possible to underflow?

Something like this would trigger an underflow and return an incorrect
result with that expression, wouldn't it?

memory_contains((void *)0x0000, (void *)0x1000, (void *)0x0, 0x1001))

AFAICS no current callers actually send in an object size which is
larger than the size of the memory, but perhaps it's best to be
defensive?

\
 
 \ /
  Last update: 2019-12-18 15:50    [W:0.851 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site