linux-kernel - Re: suppress page allocation failure warnings from sys

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]

Date:	Thu, 29 Mar 2012 16:35:56 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Joe Perches <joe@...ches.com>, Dave Jones <davej@...hat.com>,
	viro@...iv.linux.org.uk,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	David Rientjes <rientjes@...gle.com>
Subject: Re: suppress page allocation failure warnings from sys_listxattr

On Wed, Mar 28, 2012 at 06:46:02PM -0700, Andrew Morton wrote:
> On Wed, 28 Mar 2012 18:28:43 -0700 Joe Perches <joe@...ches.com> wrote:
> 
> > On Wed, 2012-03-28 at 18:10 -0700, Andrew Morton wrote:
> > > On Wed, 28 Mar 2012 20:54:42 -0400 Dave Jones <davej@...hat.com> wrote:
> > > >  > Yup.  How does the below look?
> > > > Don't see anything immediately wrong with it.
> > > > Any thoughts on what to do about the similar problem in setxattr ? (memdup_user)
> > []
> > > diff -puN fs/xattr.c~fs-xattrc-setxattr-improve-handling-of-allocation-failures fs/xattr.c
> > []
> > > @@ -334,13 +335,25 @@ setxattr(struct dentry *d, const char __
> > []
> > > +		kvalue = kmalloc(size, GFP_KERNEL | __GFP_NOWARN);
> > > +		if (!kvalue) {
> > > +			vvalue = vmalloc(size);
> > []
> > > +	if (vvalue)
> > > +		vfree(vvalue);
> > > +	else
> > > +		kfree(kvalue);
> > >  	return error;
> > 
> > These patterns are pretty common, maybe create a standard helper?
> 
> Could.  There was some discussion last year and implementations were
> tossed around.
> 
> I'm a bit apprehensive - kernel code is supposed to be robust, and
> large allocations are not robust and vmalloc() is crappy.  Formalising
> these things in an API probably won't make anything worse, but will
> deprive us of opportunities for ritualistic humiliation and
> knuckle-rapping.

I did a sweep of this recently, considering helpers for exactly such
an allocation and replacing the existing per-filesystem wrappers for
it. IIRC, there are wrappper functions in ext4, gfs2, and ntfs, XFS
now open codes it in a couple of places, there's alloc_fdmem(), cgroup
pidlists and the network code does it in several places, etc. 
Even some drivers are doing this.  It's a widespread pattern.

The easiest way to find the trivial wrappers is to grep for
is_vmalloc_addr, because all the wrapper functions use this code to
determine what to do:

	if (is_vmalloc_addr(p))
		vfree(p)
	else
		kfree(p)

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com
--
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/