Messages in this thread | | | From | Linus Torvalds <> | Date | Sat, 20 Jun 2020 09:49:47 -0700 | 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@panix.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
| |