linux-kernel - Re: [PATCH 3/3] mm/vmscan: replace implicit RECLAIM

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 for Android: free password hash cracker in your pocket

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

Message-ID: <0dd57932-44cf-0c2d-e157-07a8d7324006@intel.com>
Date:   Wed, 1 Jul 2020 14:29:38 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     David Rientjes <rientjes@...gle.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        alex.shi@...ux.alibaba.com, dwagner@...e.de, tobin@...nel.org,
        cl@...ux.com, akpm@...ux-foundation.org, ying.huang@...el.com,
        dan.j.williams@...el.com, cai@....pw
Subject: Re: [PATCH 3/3] mm/vmscan: replace implicit RECLAIM_ZONE checks with
 explicit checks

On 7/1/20 1:04 PM, Ben Widawsky wrote:
>> +static inline bool node_reclaim_enabled(void)
>> +{
>> +	/* Is any node_reclaim_mode bit set? */
>> +	return node_reclaim_mode & (RECLAIM_ZONE|RECLAIM_WRITE|RECLAIM_UNMAP);
>> +}
>> +
>>  extern void check_move_unevictable_pages(struct pagevec *pvec);
>>  
>>  extern int kswapd_run(int nid);
> If a user writes a bit that isn't a RECLAIM_* bit to vm.zone_reclaim_mode 
> today, it acts as though RECLAIM_ZONE is enabled: we try to reclaim in 
> zonelist order before falling back to the next zone in the page allocator.  
> The sysctl doesn't enforce any max value :/  I dont know if there is any 
> such user, but this would break them if there is.
> 
> Should this simply be return !!node_reclaim_mode?

You're right that there _could_ be a user-visible behavior change here.
 But, if there were a change it would be for a bit which wasn't even
mentioned in the documentation.  Somebody would have had to look at the
doc mentioning 1,2,4 and written an 8.  If they did that, they're asking
for trouble because we could have defined the '8' bit to do nasty things
like auto-demote all your memory. :)

I'll mention it in the changelog, but I still think we should check the
actual, known bits rather than check for 0.

BTW, in the hardware, they almost invariably make unused bits "reserved"
and do mean things like #GP if someone tries to set them.  This is a
case where the kernel probably should have done the same.  It would have
saved us the trouble of asking these questions now.  Maybe we should
even do that going forward.