linux-kernel - Re: [Linux-kernel-mentees] [PATCH v2] block/floppy: Prevent kernel-infoleak in raw_cmd

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]

Message-ID: <CAK8P3a2oUgdaYicdHwWvCY-HqjrcBAEzYA5yc5Gw14RLLoLdug@mail.gmail.com>
Date:   Thu, 30 Jul 2020 22:45:02 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Kees Cook <keescook@...omium.org>
Cc:     Denis Efremov <efremov@...ux.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Peilin Ye <yepeilin.cs@...il.com>,
        Jens Axboe <axboe@...nel.dk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-block <linux-block@...r.kernel.org>,
        linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] [PATCH v2] block/floppy: Prevent
 kernel-infoleak in raw_cmd_copyout()

On Thu, Jul 30, 2020 at 8:10 PM Kees Cook <keescook@...omium.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!

         Arnd