linux-kernel - Re: [PATCH] regset: use vmalloc() for regset_get

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, 1 Feb 2024 18:58:51 -0800
From: Doug Anderson <dianders@...omium.org>
To: Matthew Wilcox <willy@...radead.org>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, 
	Eric Biederman <ebiederm@...ssion.com>, Jan Kara <jack@...e.cz>, Kees Cook <keescook@...omium.org>, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org
Subject: Re: [PATCH] regset: use vmalloc() for regset_get_alloc()

Hi,

On Thu, Feb 1, 2024 at 5:24 PM Matthew Wilcox <willy@...radead.org> wrote:
>
> On Thu, Feb 01, 2024 at 05:12:03PM -0800, Douglas Anderson wrote:
> > While browsing through ChromeOS crash reports, I found one with an
> > allocation failure that looked like this:
> >
> >   chrome: page allocation failure: order:7,
> >           mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO),
> >         nodemask=(null),cpuset=urgent,mems_allowed=0
>
> That does seem bad ...
>
> > @@ -16,14 +17,14 @@ static int __regset_get(struct task_struct *target,
> >       if (size > regset->n * regset->size)
> >               size = regset->n * regset->size;
> >       if (!p) {
> > -             to_free = p = kzalloc(size, GFP_KERNEL);
> > +             to_free = p = vmalloc(size);
>
> It's my impression that sometimes this size might be relatively small?
> Perhaps we should make this kvmalloc so that we can satisfy it from the
> slab allocator if it is small?

Right. Sometimes it's small. It feels sad to me that somehow vmalloc()
of small sizes would be much less efficient than kvmalloc() of small
sizes, but I can change it to that if you want. It feels like we
should use kmalloc() if we need it to be contiguous, kvmalloc() if we
know that there will be big efficiency gains with things being
contiguous but we can get by with non-contiguous, and vmalloc() if we
just don't care. ;-)

..anyway, I'll spin v2 with kvmalloc().

> Also, I assume that we don't rely on this memory being physically
> contiguous; we don't, for example, do I/O on it?

As far as I can tell we don't. I had never looked at or thought about
this code before today and so all I have is ~an hour of code analysis
behind me, so if someone tells me I'm wrong then I'll believe them.

-Doug