linux-kernel - Re: [PATCH -tip] x86: uv - prevent NULL dereference in uv_system

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <84144f020905030954m434d0550l3ed7ef7436c803df@mail.gmail.com>
Date:	Sun, 3 May 2009 19:54:38 +0300
From:	Pekka Enberg <penberg@...helsinki.fi>
To:	Cyrill Gorcunov <gorcunov@...il.com>
Cc:	David Rientjes <rientjes@...gle.com>, Ingo Molnar <mingo@...e.hu>,
	Jack Steiner <steiner@....com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -tip] x86: uv - prevent NULL dereference in uv_system_init

On Sun, May 3, 2009 at 5:38 PM, Cyrill Gorcunov <gorcunov@...il.com> wrote:
> [Pekka Enberg - Sun, May 03, 2009 at 03:27:00PM +0300]
> | Hi Cyrill,
> |
> | On Sun, 2009-05-03 at 16:12 +0400, Cyrill Gorcunov wrote:
> | > [Pekka Enberg - Sun, May 03, 2009 at 12:59:13PM +0300]
> | > | Hi David,
> | > |
> | > | On Sun, May 3, 2009 at 12:09 PM, David Rientjes <rientjes@...gle.com> wrote:
> | > | > SLUB stores two new slab allocation orders: the cache's adjustable order
> | > | > which is calculated at kmem_cache_create(), and the smallest order that
> | > | > can accommodate at least one object allocation.  The latter is used as a
> | > | > fallback when the former fails in the page allocator.
> | > | >
> | > | > So for __GFP_PANIC to work in this case, it could not be implemented in
> | > | > the page allocator (SLUB also passes __GFP_NORETRY for new slabs) but
> | > | > rather above it in allocate_slab().  It would then be a no-op for
> | > | > alloc_pages().
> | > |
> | > | It's probably better to implement __GFP_PANIC in alloc_pages() because
> | > | of kmalloc_large(). You can easily mask the __GFP_PANIC from the first
> | > | call to alloc_slab_page() where we use __GFP_NOWARN to suppress
> | > | out-of-memory warnings.
> | > |
> | > | But anyway, enough talk, show me the patch! :-)
> | > |
> | > |                                           Pekka
> | > |
> | >
> | > I was thinking about the approach showed below.
> | >
> | > Note even if we will agree on this idea a number
> | > of questions remain opened -- like where is a better
> | > place to define kmalloc_panic in slub/slab_def.h
> | > or rather in slab.h. Should we include kernel.h
> | > to have panic and pr_ properly defined?
> | >
> | > I don't dare start/introduce handling of __GFP_PANIC
> | > flag since it would require more efforts to be done
> | > correctly and what is more important -- for most
> | > cases we would just don't need it.
> | >
> | >     -- Cyrill
> | >
> | > ---
> | >  include/linux/slab_def.h |   12 ++++++++++++
> | >  1 file changed, 12 insertions(+)
> | >
> | > Index: linux-2.6.git/include/linux/slab_def.h
> | > =====================================================================
> | > --- linux-2.6.git.orig/include/linux/slab_def.h
> | > +++ linux-2.6.git/include/linux/slab_def.h
> | > @@ -220,4 +220,16 @@ found:
> | >
> | >  #endif     /* CONFIG_NUMA */
> | >
> | > +static inline void *kmalloc_panic(size_t size, gfp_t flags)
> | > +{
> | > +   void *p = kmalloc(size, flags);
> | > +
> | > +   if (size && ZERO_OR_NULL_PTR(p)) {
> | > +           pr_emerg("Failed to allocate: %z bytes\n", size);
> | > +           panic("Out of memory\n");
> | > +   }
> | > +
> | > +   return p;
> | > +}
> | > +
> | >  #endif     /* _LINUX_SLAB_DEF_H */
> |
> | I don't like this approach because you'd need to do a kzalloc_panic()
> | and so on for it to be truly useful. What's wrong with adding a
> | __GFP_PANIC check in __alloc_pages_internal() (or whatever it's called
> | in -mm now) next to __GFP_NOWARN?
> |
> |                       Pekka
> |
>
> Hi Pekka,
>
> ufortunatelly __alloc_pages_internal is not the only place where
> we do return NULL from kmalloc. As example - failslab facility
> (in slab_alloc call). Anyway -- I'll take a closer look.

Right. I think failslab needs some fixing _not_ to return NULL if
__GFP_PANIC is set.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/