Am 05.11.20 um 17:37 schrieb Daniel Vetter:
[SNIP]
NAK, we use this to determine if a pages belongs to the driver or not in
amdgpu for example.
Mostly used for debugging, but I would really like to keep that.
Can you pls point me at that code? A quick grep hasn't really found much at all.
See amdgpu_iomem_read() for an example:
Why do you reject this?
When IOMMU is disabled or uses an 1 to 1 mapping we would otherwise give the
same access as /dev/mem to system memory and that is forbidden. But as I
noted this is just for the debugfs file.
Ah, there's a config option for that. Plus it's debugfs, anything goes in
debugfs, but if you're worried about that hole we should just disable the
entire debugfs file for CONFIG_STRICT_DEVMEM. I can perhaps throw that on
top, that follow_pfn patch series I'm baking is all about this kind of
fun.
And exactly that would get a NAK from us.
We have specially created that debugfs file as an alternative when
CONFIG_STRICT_DEVMEM is set.
Uh that doesn't work if you work around core restrictions with your
own debugfs paths.
That's why we have the restriction to check the mapping of the pages.
This way we only expose the memory which was allocated by our driver and
don't allow any uncontrolled access to the whole system memory.
We have something similar for radeon as well, but there we have a global
GART table which we can use for validating stuff.
The check doesn't take any locks over the check and copy*user, I don't
think it's protecting anything really against somewhat adverse
userspace.
You mean that it's racing with freeing the pages? Yes that is an obvious
problem, but we already had the same issue for radeon as well.
On the other hand we could easily prevent that with a lock.
I mean fundamentally locking down stuff like STRICT_DEVMEM or all the
others makes debugging harder, that's kinda the expected tradeoff.
Well we just wanted to have the same debugging functionality we had for
radeon.
As you said debugfs is a rabbit hole regarding attack vectors anyway.
If you are root you can just go to /sys and start reprogramming the
hardware to do what you want to do.
Regards,
Christian.
Maybe you can do fun like this in your dkms, but
not in upstream. Like if this was specifically created to work around
CONFIG_STRICT_DEVMEM (and it sounds like that) then I think this
should never have landed in upstream to begin with.
I'm also kinda confused that there's distros with CONFIG_STRICT_DEVMEM
which allow debugfs. debugfs is a pretty bad root hole all around, or
at least that's been the assumption all the time.
Yeah, completely agree :) But that's not my problem.
I guess I'll do another rfc series and poke a pile of people ... seems
to be a habit I'm developing :-)
-Daniel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
