Re: [PATCH v2] drm/amdgpu: Gate debugfs MMIO access on kernel lockdown

[Lazar, Lijo]

On 02-Jun-26 11:48 PM, Asad Kamal wrote:
amdgpu_regs, amdgpu_regs2, and related debugfs nodes allow arbitrary
MMIO read/write via RREG32/WREG32 without checking security_locked_down().
On kernel_lockdown=integrity systems this bypasses the same restrictions
as /dev/mem and PCI config space sysfs.

Check LOCKDOWN_PCI_ACCESS (matching pci-sysfs) at the entry of every
debugfs handler that performs direct register access

v2: Use consistent check as per previous check to use
LOCKDOWN_DEBUGFS(Lijo)


I didn't mean to apply LOCKDOWN_DEBUGFS. Debugfs is locked down 
internally for all writes in LOCKDOWN_DEBUGFS.

https://github.com/torvalds/linux/blob/master/fs/debugfs/file.c#L267

So this patch may not help much for files opened for write operations -

https://patchwork.freedesktop.org/patch/728769/?series=167435&rev=1

Intention of the comment was to decide between 
LOCKDOWN_PCI_ACCESS/LOCKDOWN_MMIOTRACE or something related to device 
access and use it consistently.

Thanks,
Lijo

Signed-off-by: Asad Kamal <asad.ka...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 28 +++++++++++++++++++++
  1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 0ce6e2e4342c..c554afa0963d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -26,6 +26,7 @@
  #include <linux/kthread.h>
  #include <linux/pci.h>
  #include <linux/uaccess.h>
+#include <linux/security.h>
  #include <linux/pm_runtime.h>
  
  #include "amdgpu.h"
@@ -86,6 +87,9 @@ static int  amdgpu_debugfs_process_reg_op(bool read, struct 
file *f,
                        ((*pos & (1ULL << 62)) && (*pos & (1ULL << 61))))
                return -EINVAL;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        /* are we reading registers for which a PG lock is necessary? */
        pm_pg_lock = (*pos >> 23) & 1;
  
@@ -237,6 +241,9 @@ static ssize_t amdgpu_debugfs_regs2_op(struct file *f, char __user *buf, u32 off
        if (size & 0x3 || offset & 0x3)
                return -EINVAL;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
        if (r < 0) {
                pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -402,6 +409,9 @@ static ssize_t amdgpu_debugfs_gprwave_read(struct file *f, 
char __user *buf, siz
        if (size > 4096 || size & 0x3 || *pos & 0x3)
                return -EINVAL;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
        if (r < 0) {
                pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -521,6 +531,9 @@ static ssize_t amdgpu_debugfs_regs_pcie_read(struct file 
*f, char __user *buf,
        if (size & 0x3 || *pos & 0x3)
                return -EINVAL;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
        if (r < 0) {
                pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -635,6 +648,9 @@ static ssize_t amdgpu_debugfs_regs_pcie64_read(struct file 
*f, char __user *buf,
        if (size & 0x7 || *pos & 0x7)
                return -EINVAL;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
        if (r < 0) {
                pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -746,6 +762,9 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct file 
*f, char __user *buf,
        if (!adev->reg.didt.rreg)
                return -EOPNOTSUPP;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
        if (r < 0) {
                pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -863,6 +882,9 @@ static ssize_t amdgpu_debugfs_regs_smc_read(struct file *f, 
char __user *buf,
        if (size & 0x3 || *pos & 0x3)
                return -EINVAL;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
        if (r < 0) {
                pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -1156,6 +1178,9 @@ static ssize_t amdgpu_debugfs_wave_read(struct file *f, 
char __user *buf,
        if (size & 3 || *pos & 3)
                return -EINVAL;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        /* decode offset */
        offset = (*pos & GENMASK_ULL(6, 0));
        se = (*pos & GENMASK_ULL(14, 7)) >> 7;
@@ -1247,6 +1272,9 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, 
char __user *buf,
        if (size > 4096 || size & 3 || *pos & 3)
                return -EINVAL;
  
+	if (security_locked_down(LOCKDOWN_DEBUGFS))
+               return -EPERM;
+
        /* decode offset */
        offset = (*pos & GENMASK_ULL(11, 0)) >> 2;
        se = (*pos & GENMASK_ULL(19, 12)) >> 12;