linux-kernel - Re: [PATCH 1/2] cpumask: Introduce possible_cpu

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, 4 Apr 2019 13:28:05 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Dan Carpenter <dan.carpenter@...cle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Jens Axboe <axboe@...nel.dk>,
        Amritha Nambiar <amritha.nambiar@...el.com>,
        Willem de Bruijn <willemb@...gle.com>,
        kernel-janitors@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] cpumask: Introduce possible_cpu_safe()

On Thu, Apr 04, 2019 at 12:35:28PM +0200, Michal Hocko wrote:
> On Thu 04-04-19 13:02:19, Dan Carpenter wrote:
> > There have been two cases recently where we pass user a controlled "cpu"
> > to possible_cpus().  That's not allowed.  If it's invalid, it will
> > trigger a WARN_ONCE() and an out of bounds read which could result in an
> > Oops.
> > 
> > This patch introduces possible_cpu_safe() which first checks to see if
> > the cpu is valid, turns off speculation and then checks if the cpu is
> > possible.
> 
> Why cannot we do the check in possible_cpu directly? Is it used from any
> hot path? I am quite skeptical people will use the new helper
> consistently.

Why only possible? What is to say stop anyone from using garbage (aka.
user input) in any other of the cpumask APIs.

I'd much rather have the explicit validate call and keep assuming @cpu
as used in the rest of the API is sane.