linux-kernel - Re: [patch] mm, thp: fix defrag setting if newline is not used

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]

Date:   Fri, 17 Jan 2020 11:12:24 +0100
From:   Vlastimil Babka <vbabka@...e.cz>
To:     David Rientjes <rientjes@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Mel Gorman <mgorman@...hsingularity.net>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [patch] mm, thp: fix defrag setting if newline is not used

On 1/17/20 10:43 AM, David Rientjes wrote:
> On Fri, 17 Jan 2020, Vlastimil Babka wrote:
> 
>>>> If thp defrag setting "defer" is used and a newline is *not* used when
>>>> writing to the sysfs file, this is interpreted as the "defer+madvise"
>>>> option.
>>>>
>>>> This is because we do prefix matching and if five characters are written
>>>> without a newline, the current code ends up comparing to the first five
>>>> bytes of the "defer+madvise" option and using that instead.
>>>>
>>>> Find the length of what the user is writing and use that to guide our
>>>> decision on which string comparison to do.
>>>
>>> Gee, why is this code so complicated?  Can't we just do
>>>
>>> 	if (sysfs_streq(buf, "always")) {
>>> 		...
>>> 	} else if sysfs_streq(buf, "defer+madvise")) {
>>> 		...
>>> 	}
>>> 	...
>>
>> Yeah, if we knew this existed :)
>>
>> We would lose the prefix matching but hopefully nobody will complain.
>>
> 
> I tested Vlastimil's patch and it works as intended so I was about to 
> modify the changelog and send his patch and ask for a sign-off line 
> because I think I agree the *partial* prefix matching has ~0.1% chance of 
> breaking userspace and that 0.1% chance outweighs my desire to make the 
> code consistent for all options.

If prefix matching worked with "echo alw > /sys..." then I would expect
some script out there relies on it, but since it only works with "echo
-n alw > /..." then perhaps there's no such script :)

> But if userspace were broken by this, then at least it was already broken 
> for "defer" depending on newline vs no newline.  (What we do know is that 
> nobody has used "defer" for the past couple years without a newline :).
> 
> If nobody objects, I'll test and send Andrew's version with the changelog 
> because I think we all agree the risk of breakage here is very minimal and 
> actually fixes the case for defer.  

Agreed.