Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 23 Jul 2021 15:22:12 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     Denis Efremov <efremov@linux.com>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        Peilin Ye <yepeilin.cs@gmail.com>,
        Jens Axboe <axboe@kernel.dk>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        linux-block <linux-block@vger.kernel.org>,
        linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] [PATCH v2] block/floppy: Prevent
 kernel-infoleak in raw_cmd_copyout()
Message-ID: <202107231520.32B389411@keescook>
References: <20200728141946.426245-1-yepeilin.cs@gmail.com>
 <20200729115157.8519-1-yepeilin.cs@gmail.com>
 <20200729125820.GB1840@kadam>
 <f2cf6137-987a-ab41-d88a-6828d46c255f@linux.com>
 <CAK8P3a20SEoYCrp3jOK32oZc9OkiPv+1KTjNZ2GxLbHpY4WexQ@mail.gmail.com>
 <202007301056.D3BD1805B0@keescook>
 <CAK8P3a2oUgdaYicdHwWvCY-HqjrcBAEzYA5yc5Gw14RLLoLdug@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAK8P3a2oUgdaYicdHwWvCY-HqjrcBAEzYA5yc5Gw14RLLoLdug@mail.gmail.com>
Precedence: bulk

On Thu, Jul 30, 2020 at 10:45:02PM +0200, Arnd Bergmann wrote:
> On Thu, Jul 30, 2020 at 8:10 PM Kees Cook <keescook@chromium.org> wrote:
> > On Thu, Jul 30, 2020 at 10:11:07AM +0200, Arnd Bergmann wrote:
> >
> > test_stackinit.c intended to use six cases (where "full" is in the sense
> > of "all members are named", this is intentionally testing the behavior
> > of padding hole initialization):
> 
> Ok, so I read that correctly, thanks for confirming.
> 
> > >
> > >    struct test_big_hole var = *arg;
> >
> > So this one is a "whole structure copy" which I didn't have any tests
> > for, since I'd (perhaps inappropriately) assumed would be accomplished
> > with memcpy() internally, which means the incoming "*arg"'s padding holes
> > would be copied as-is. If the compiler is actually doing per-member copies
> > and leaving holes in "var" untouched, that's unexpected, so clearly that
> > needs to be added to test_stackinit.c! :)
> 
> For some reason I remembered this not turning into a memcpy()
> somewhere, but I can't reproduce it in any of my recent attempts,
> just like what Denis found.
> 
> > > or the a constructor like
> > >
> > >   struct test_big_hole var;
> > >   var = (struct test_big_hole){ .one = arg->one, .two=arg->two, .three
> > > = arg->three, .four = arg->four };
> > >
> > > Kees, do you know whether those two would behave differently?
> > > Would it make sense to also check for those, or am I perhaps
> > > misreading your code and it already gets checked?
> >
> > I *think* the above constructor would be covered under "full runtime
> > init", but it does also seem likely it would be handled similarly to
> > the "whole structure copy" in the previous example.
> 
> I would assume that at least with C99 it is more like the
> "whole structure copy", based on the standard language of
> 
>   "The value of the compound literal is that of an unnamed
>   object initialized by the initializer list. If the compound literal
>   occurs outside the body of a function, the object has static
>   storage duration; otherwise, it has automatic storage duration
>   associated with the enclosing block."
> 
> > I will go add more tests...
> 
> Thanks!

Well, nearly exactly a year later, I've finally done this. :P

https://lore.kernel.org/lkml/20210723221933.3431999-1-keescook@chromium.org

-- 
Kees Cook