-
Notifications
You must be signed in to change notification settings - Fork 154
libdmabufheap: Add udev rules for dmabuf heaps #1088
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
libdmabufheap: Add udev rules for dmabuf heaps #1088
Conversation
8449efd to
6831e3a
Compare
|
What installs this file? This PR only adds the file, but doesn't change any of the recipes to use it. |
Add udev rules to let non-root userspace apps access dmabuf heaps. Signed-off-by: Praveen Kumar <[email protected]>
6831e3a to
41f40b7
Compare
Sorry, missed it earlier. Recipe is added now. |
|
The commit looks OK to me, but is this something we should do globally? Is unrestricted DMA buff access a thing we want? |
| @@ -0,0 +1,2 @@ | |||
| ACTION=="add" SUBSYSTEM=="dma_heap", KERNEL=="system" GROUP="kmem", MODE="0640" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is 'kmem' group? it's not defined anywhere.
on any standard linux system (my laptop or a corporate server), /dev/dma_heap/system is owned by root:root and with 600 permission. We probably want to think twice before opening it up like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was one of the issues with libcamera: it also needed dma-heap access.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The kmem group is a reserved group in Linux with a fixed group ID of 15, traditionally used for memory-related device access. While it may not be explicitly defined on all standard Linux distributions (like laptops or servers), it is recognized in embedded and Android-based systems for managing access to memory-related device nodes.
We're aligning with the QLI Standard variant where downstream groups like system are not available. To ensure consistent access across containers and clients (e.g., fastrpc, video, etc.), we're moving /dev/dma_heap/system and similar nodes to the kmem group. This allows non-root applications to access these nodes securely by subscribing to the kmem group as a supplementary group.
Using 640 with controlled group access via kmem also ensures better isolation and compliance with security guidelines.
For reference: [BUG] Could not open any dmaHeap device · Issue #218 · raspberrypi/rpicam-apps => here they used "video" group instead of "kmem" group
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I found:
kmem: /dev/mem and /dev/port are readable by this group. This is mostly a BSD relic, but any programs that need direct read access to the system's memory can thus be made SETGID kmem. /dev/kmem, that has given the name to this group, was also readable by this group, but is currently disabled by default for security reasons.
on https://wiki.debian.org/SystemGroups. And I can see this system group exists (in debian/ubuntu), I haven't checked OE/yocto.
That said it is not obvious that kmem is the right group to use for /dev/dma_heap. And if it is the right thing to do, we need the udev rules in a better place upstream than here anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's created here
https://github.com/systemd/systemd/blob/main/rules.d/50-udev-default.rules.in#L46
And Debian/Ubuntu have that too:
https://salsa.debian.org/systemd-team/systemd/-/blob/debian/master/debian/extra/rules/80-debian-compat.rules?ref_type=heads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@praveenkumar-oss there is lack of consensus, and it's a fragmented space: each distro has a different way of creating the default groups, there is no single upstream for this list. This creates a lot of inertia between requesting a new group and being able to trust it will exist by default with a stable gid across distros.
Adding users to the root group sounds wrong too though. I do think you're on the right track, and you should think of a system architecture that let's a particular container get access to a privileged resource.
If I'm an end-user in a desktop session, I get access to specific devices through FACLs on the relevant device nodes, or I use a middleware such as bluez to mediate access. How would it look like here?
We could for instance:
- dynamically create a group, and add some user that wants to run the workload to that group; we could provide a sample setup script that helps with that and a runtime script that checks the environment is good at runtime
- or mediate access in a middleware, not sure which one would be applicable; we could pass sockets / fds to gain access inside containers
- your own idea here
I think it's important to keep the privileges of the containers/workloads minimal as to be able to run apps/workloads as non-root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is 'kmem' group? it's not defined anywhere.
on any standard linux system (my laptop or a corporate server), /dev/dma_heap/system is owned by root:root and with 600 permission. We probably want to think twice before opening it up like that.
@lool I was referring to this comment that root is already in use by distro. What I am suggesting is to use the same what's already there to unblock the user, and in parallel, a better solution could be explored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better solution is:
- Stop using DMA heaps. They are not enabled by default in most of the distros, they are not subject to cgroups, etc.
- Use udmabuf + memfd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stop using DMA heaps.
? .. tmk, most of our clients used the system heap as a means to allocate and share the buffers and asking them to stop using it AND switch to udmabuf + memfd will not be well received. Infact, as you know, system heap is performant than udmabuf + memfd because of the way it allocate the higher order pages(and can easily extended to using the pools).... where as the former is loosely relied on the THP + 4K pages.
Totally understood about your comment, but system heap is not the only heap that the client uses, there are like cma heap and protected heap(in future), for which the same problem statement exist and there is no equivalent solution, I guess.
They are not enabled by default in most of the distros
Just curious about how did you check this for which are all distros it is enabled? Manually ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@charan-kalla-oss you can revive the discussion with the upstream. I'm basically following the advice that everyone got last year.
They are not enabled by default in most of the distros
Just curious about how did you check this for which are all distros it is enabled? Manually ?
Previously I checked several important distros manually. I see that since that time Fedora has enabled DMAHEAPS support. Neither Yocto (linux-yocto kernels) nor Debian (trixie, forky) have them enabled. So... It's still a very questionable dependency.
| SRCREV = "5ead341efb667cbd2cf519bfaf2837ee8e97b747" | ||
| SRC_URI = " \ | ||
| git://git.codelinaro.org//clo/le/platform/system/memory/libdmabufheap.git;branch=memory-le-apps.lnx.3.0;protocol=https \ | ||
| file://dmaheap.rules \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this please go upstream so that it's picked up by all distros rather than just this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one of the problem with this recipe, is that there is no real upstream.. at least not a clean upstream plan/strategy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
systemd-udev would be a correct upstream. I think, the permissions on the dma-heaps are a generic issue that should be solved for all the systems, not just Qualcomm.
lumag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please work with dri-devel@ and linux-media@ to identify a proper group assignment and a proper upstream project to land this change.
Add udev rules to let non-root userspace apps access dmabuf heaps.