linux-kernel - Re: [PATCH] dm ioctl: Restore __GFP_HIGH in copy

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-next>] [day] [month] [year] [list]

Date:   Thu, 18 May 2017 19:50:46 -0700
From:   Junaid Shahid <junaids@...gle.com>
To:     David Rientjes <rientjes@...gle.com>
Cc:     Michal Hocko <mhocko@...nel.org>, Alasdair Kergon <agk@...hat.com>,
        Mike Snitzer <snitzer@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        andreslc@...gle.com, gthelen@...gle.com, mpatocka@...hat.com,
        vbabka@...e.cz, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dm ioctl: Restore __GFP_HIGH in copy_params()

(Adding back the correct linux-mm email address and also adding linux-kernel.)

On Thursday, May 18, 2017 01:41:33 PM David Rientjes wrote:
> On Thu, 18 May 2017, Michal Hocko wrote:
> 
> > On Thu 18-05-17 11:50:40, Junaid Shahid wrote:
> > > d224e9381897 (drivers/md/dm-ioctl.c: use kvmalloc rather than opencoded
> > > variant) left out the __GFP_HIGH flag when converting from __vmalloc to
> > > kvmalloc. This can cause the IOCTL to fail in some low memory situations
> > > where it wouldn't have failed earlier. This patch adds it back to avoid
> > > any potential regression.
> > 
> > The code previously used __GFP_HIGH only for the vmalloc fallback and
> > that doesn't make that much sense with the current implementation
> > because vmalloc does order-0 pages and those do not really fail and the
> > oom killer is invoked to free memory.
> > 
> 
> Order-0 pages certainly do fail, there is not an infinite amount of memory 
> nor is there a specific exemption to allow order-0 memory to be alloctable 
> below watermarks without this gfp flag.  OOM kill is the last thing we 
> want for these allocations since they are very temporary.
> 
> > There is no reason to access memory reserves from this context.
> > 
> 
> Let's ask Mikulas, who changed this from PF_MEMALLOC to __GFP_HIGH, 
> assuming there was a reason to do it in the first place in two different 
> ways.
> 
> This decision is up to the device mapper maintainers.
> 
> > > Signed-off-by: Junaid Shahid <junaids@...gle.com>
> > > ---
> > >  drivers/md/dm-ioctl.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> > > index 0555b4410e05..bacad7637a56 100644
> > > --- a/drivers/md/dm-ioctl.c
> > > +++ b/drivers/md/dm-ioctl.c
> > > @@ -1715,7 +1715,7 @@ static int copy_params(struct dm_ioctl __user *user, struct dm_ioctl *param_kern
> > >  	 */
> > >  	dmi = NULL;
> > >  	noio_flag = memalloc_noio_save();
> > > -	dmi = kvmalloc(param_kernel->data_size, GFP_KERNEL);
> > > +	dmi = kvmalloc(param_kernel->data_size, GFP_KERNEL | __GFP_HIGH);
> > >  	memalloc_noio_restore(noio_flag);
> > >  
> > >  	if (!dmi) {