linux-kernel - Re: [PATCH -mm] mm/page_isolation: fix potential warning from user

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: <20200120074222.GF18451@dhcp22.suse.cz>
Date:   Mon, 20 Jan 2020 08:42:22 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Anshuman Khandual <anshuman.khandual@....com>
Cc:     Qian Cai <cai@....pw>, akpm@...ux-foundation.org, david@...hat.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -mm] mm/page_isolation: fix potential warning from user

On Mon 20-01-20 09:50:33, Anshuman Khandual wrote:
> Hello Qian,
> 
> On 01/20/2020 09:12 AM, Qian Cai wrote:
> > It makes sense to call the WARN_ON_ONCE(zone_idx(zone) == ZONE_MOVABLE)
> > from the offlining path, but should avoid triggering it from userspace,
> > i.e, from is_mem_section_removable().
> 
> Could you elaborate why it makes sense not to warn about an unmovable
> ZONE_MOVABLE page when an user tries to query about a memory block
> device's movability through sysfs ?

Because somebody might have panic_on_warn and then this is unlikely (but
not impossible) way to put the system down by arbitrary user. Besides
that it is stupid to warn when we convey the information to the
userspace anyway.
 
[...]
> > +	} else {
> > +		if (isol_flags & MEMORY_OFFLINE)
> > +			WARN_ON_ONCE(zone_idx(zone) == ZONE_MOVABLE);> +
> > +		if ((isol_flags & REPORT_FAILURE) && !IS_ERR(unmovable))
> > +			/*
> > +			 * printk() with zone->lock held will likely trigger a
> > +			 * lockdep splat, so defer it here.
> > +			 */
> > +			dump_page(unmovable, "unmovable page");
> > +	}
> > +
> > +	return !!unmovable;
> >  }
> >  
> >  static void unset_migratetype_isolate(struct page *page, unsigned migratetype)
> 
> set_migratetype_isolate() gets called from CMA as well as HugeTLB
> allocation paths, so its not only during offline. Hence the commit
> message should be changed to reflect this.

We should just report for all those cases I believe.
-- 
Michal Hocko
SUSE Labs