linux-kernel - Re: Commit 25f12ae45fc1 ("maccess: rename probe_kernel_address to get_kernel

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: <CAHk-=whj7YBvNT3FPHc8oUqwRhjbRkJESnUx6bbpA5ys6W9ujw@mail.gmail.com>
Date:   Sat, 20 Jun 2020 09:49:47 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Kenneth R. Crudup" <kenny@...ix.com>
Cc:     Christoph Hellwig <hch@....de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Commit 25f12ae45fc1 ("maccess: rename probe_kernel_address to
 get_kernel_nofault") causing several OOPSes

On Sat, Jun 20, 2020 at 6:46 AM Kenneth R. Crudup <kenny@...ix.com> wrote:
>
> So, be totally surprised :) I've just booted with "maccess: rename
> probe_kernel_address to get_kernel_nofault" intact and your probe_roms.c
> patch with no issues.
>
> (Perhaps there's some sort of compiler optimization going on?)

Hmm.

Very strange. I was a tiny bit worried about that part of the patch,
because I also changed the types (from "unsigned char *" to "void *"),
but pointer arithmetic in "unsigned char *" and "void *" is the same,
and Christoph's partial revert patch doesn't even revert that part.

But I really don't see what Christoph's revert would really even
change It switches the order of the arguments back..

It does re-introduce a bug in that macro that I fixed. This macro is
buggy garbage:

+#define probe_kernel_address(addr, retval)             \
+       copy_from_kernel_nofault(&retval, addr, sizeof(retval))

in case 'retval' is a complex expression, becasue of possibly changing
the C order of operations. So it needs to be "&(retval)" in the macro
body.

But that is never the case for 'retval'. For 'addr', yes, but 'addr'
is only used simply (and copy_from_kernel_nofault() isn't a macro).

I'm staring at that opatch and not seeing how it could _possibly_ make
any difference in code generation.

Which is the obvious next step: would you mind compiling that file
with and without the patch and sending me the two object files?

              Linus