Messages in this thread | | | From | Linus Torvalds <> | Date | Tue, 2 May 2023 13:14:33 -0700 | Subject | Re: [GIT PULL] x86/mm for 6.4 |
| |
On Tue, May 2, 2023 at 9:00 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > I guess it also wouldn't matter as much either if we hid it in a helper > > like the attached patch and I didn't have to read it twice. ;) > > Yeah, I think that's a good solution.
Hmm. And as I was rebasing the patch to fix up my patch, I realized that the current -git top-of-tree state is actually broken.
That
#define access_ok(addr, size) \ ({ \ WARN_ON_IN_IRQ(); \ likely(__access_ok(untagged_addr(addr), size)); \ })
is actually *wrong* in two ways.
Now, in my original patch, I added a comment about how that "WARN_ON_IN_IRQ()" is bogus and this shouldn't be x86-specific at all.
I ended up going back in time to see why it was added, and I think it was added because we used to access 'current' in access_ok(), due to it using that user_addr_max() thing:
likely(!__range_not_ok(addr, size, user_addr_max()));
but that was all removed by the set_fs() removal by Christoph Hellwig.
So there is actually nothing task-related in "access_ok()" any more, and any warning about using it in the wrong context is just bogus. That warning simply shouldn't exist any more (or maybe it should be in a different place, like the actual user copy functions)
But that's actually not the problem with the above.
No, the problem is that probably *because* "access_ok()" has that warning, not all users use "access_ok()" at all. We have places that use "__access_ok()" instead. Like copy_from_nmi().
So now copy_from_nmi() doesn't do the untagging, so if you were to use tagged pointers for the stack, you'd not get stack traces.
End result: I think that
(a) that WARN_ON_IN_IRQ() is actively detrimental and causes problems
(b) the current "use untagged_addr() in access_ok()" model is also broken
and my patch - which was meant to just improve code generation - actually fixes this.
Linus
| |