linux-kernel - Re: [v3 PATCH] mm: move_pages: return valid node id in status if the page is already on the target node

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, 5 Dec 2019 18:58:20 -0500
From:   Qian Cai <cai@....pw>
To:     John Hubbard <jhubbard@...dia.com>
Cc:     Yang Shi <yang.shi@...ux.alibaba.com>, fabecassis@...dia.com,
        mhocko@...e.com, cl@...ux.com, vbabka@...e.cz,
        mgorman@...hsingularity.net, akpm@...ux-foundation.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [v3 PATCH] mm: move_pages: return valid node id in status if the page is already on the target node



> On Dec 5, 2019, at 6:24 PM, John Hubbard <jhubbard@...dia.com> wrote:
> 
> Let's check in the fix that is clearly correct and non-controversial, in one
> patch. Then another patch can be created for the other case. This allows forward
> progress and quick resolution of the user's bug report, while still dealing
> with all the problems.
> 
> If you try to fix too many problems in one patch (and remember, sometimes ">1"
> is too many), then things bog down. It's always a judgment call, but what's 
> unfolding here is quite consistent with the usual judgment calls in this area.
> 
> I don't think anyone is saying, "don't work on the second problem", it's just
> that it's less urgent, due to no reports from the field. If you are passionate
> about fixing the second problem (and are ready and willing to handle the fallout
> from user space, if it occurs), then I'd encourage you to look into it.
> 
> It could turn out to be one of those "cannot change this because user space expectations
> have baked and hardened, and changes would break user space" situations, just to
> warn you in advance, though.

There is no need to paper over the underlying issue. One can think there is only one problem. The way move_pages() deal with pages are already in the desired node. Then, I don’t see there is any controversy that it was broken for so long and just restore it to according to the manpage. If you worried about people has already depended on the broken behavior, it could stay in linux-next for many releases to gather feedback. In any case, I don’t see it need to hurry to fix this until someone can show the real world use case for it apart from some random test code.