Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200302130430.201037-2-glider@google.com> <0eaac427354844a4fcfb0d9843cf3024c6af21df.camel@perches.com>
 <CAG_fn=VNnxjD6qdkAW_E0v3faBQPpSsO=c+h8O=yvNxTZowuBQ@mail.gmail.com>
 <4cac10d3e2c03e4f21f1104405a0a62a853efb4e.camel@perches.com>
 <CAG_fn=XOyPGau9m7x8eCLJHy3m-H=nbMODewWVJ1xb2e+BPdFw@mail.gmail.com>
 <18b0d6ea5619c34ca4120a6151103dbe9bfa0cbe.camel@perches.com>
 <CAG_fn=U2T--j_uhyppqzFvMO3w3yUA529pQrCpbhYvqcfh9Z1w@mail.gmail.com>
 <20200303093832.GD24372@kadam> <202003040951.7857DFD936@keescook>
 <20200305080756.GB19839@kadam> <202003050010.A1A965BF37@keescook>
In-Reply-To: <202003050010.A1A965BF37@keescook>
From:   Alexander Potapenko <glider@google.com>
Date:   Thu, 5 Mar 2020 09:33:07 +0100
Message-ID: <CAG_fn=WSw0Wt=mJs5Vm_133g0--i0yMOTsuEQ-9YbmLo77+w_A@mail.gmail.com>
Subject: Re: [PATCH v2 2/3] binder: do not initialize locals passed to copy_from_user()
To:     Kees Cook <keescook@chromium.org>
Cc:     Dan Carpenter <dan.carpenter@oracle.com>,
        "open list:ANDROID DRIVERS" <devel@driverdev.osuosl.org>,
        Jann Horn <jannh@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        LKML <linux-kernel@vger.kernel.org>,
        =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= <arve@android.com>,
        Ingo Molnar <mingo@redhat.com>, Joe Perches <joe@perches.com>,
        Dmitriy Vyukov <dvyukov@google.com>,
        Todd Kjos <tkjos@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 5, 2020 at 9:26 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Mar 05, 2020 at 11:07:56AM +0300, Dan Carpenter wrote:
> > On Wed, Mar 04, 2020 at 10:13:40AM -0800, Kees Cook wrote:
> > > On Tue, Mar 03, 2020 at 12:38:32PM +0300, Dan Carpenter wrote:
> > > > The real fix is to initialize everything manually, the automated
> > > > initialization is a hardenning feature which many people will disable.
> > >
> > > I cannot disagree more with this sentiment. Linus has specifically said he
> > > wants this initialization on by default[1],
> >
> > Fine, but as long as it's a configurable thing then we need to manually
> > initialize as well or it's still a CVE etc.  It will take a while before
> > we drop support for old versions of GCC as well.
>
> Yes, I agree; that's totally true. We need to continue to fix all the
> uninitialized flaws we encounter unless this is on by default for all
> supported compiler versions (which will be a looong time). (But it's
> not relevant to this patch because copy_from_user() does already do
> the initialization.)
>
> This set of patches was about dealing with the pathological cases of
> auto-init colliding with functions that do, in fact, fully init. Though
> I must say, I remain concerned about inventing such markings for fear
> they'll be used in places where the "trust me, it's fully initialized"
> state does not actually hold[1] but the author thinks it does.
>
> -Kees

Right now I'm trying to make Clang understand that output arguments of
inline assembly initialize the memory.
Then it would be possible to write something like:

  struct binder_transaction_data tr;
  asm("": "=m"(tr));
  if (copy_from_user(&tr, ptr, sizeof(tr))) ...

, and the asm directive can be hidden into copy_from_user().