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

[Nicolas Bouchinet <nicolas.bouchinet@oss.cyber.gouv.fr>]

On Mon, Jun 08, 2026 at 11:03:05AM +0000, Kamal, Asad wrote:
> AMD General
> 
> Hi @Nicolas Bouchinet
> 
> Thank you for the review.
> 
> The commit message references v1 behavior and is no longer accurate. Writes are already blocked, the existing debugfs_locked_down() in fs/debugfs/file.c handles writes when FMODE_WRITE is set, the early-return guard fails and security_locked_down(LOCKDOWN_DEBUGFS) blocks the open under [integrity].
> 
> The patch addresses only the read path. Here is why reads are not blocked by debugfs_locked_down():
> 
> static int debugfs_locked_down(struct inode *inode,
>                                struct file *filp,
>                                const struct file_operations *real_fops)
> {
>     if ((inode->i_mode & 07777 & ~0444) == 0 &&
>         !(filp->f_mode & FMODE_WRITE) &&
>         (!real_fops || (!real_fops->unlocked_ioctl &&
>                         !real_fops->compat_ioctl &&
>                         !real_fops->mmap)))
>         return 0;
>     if (security_locked_down(LOCKDOWN_DEBUGFS))
>         return -EPERM;
>     return 0;
> }
> 
> For a read-only open of amdgpu_regs (mode 0400, no ioctl, no mmap):
> 
> 1) (0400 & 07777 & ~0444) == 0 → true - any mode with no bits set outside the 0444 mask (e.g. 0400, 0440, 0444) 0400 satisfies that.
> 2) !(filp->f_mode & FMODE_WRITE) → true for a read-only open.
> 3) No unlocked_ioctl, compat_ioctl, or mmap in amdgpu_debugfs_regs_fops → true.
> 
> All three conditions hold, so debugfs_locked_down() returns 0 and the read open proceeds. The read handler then calls RREG32, a direct hardware MMIO read, with no further lockdown check. That is the gap which this patch is addressing.
> 
> Thanks & Regards
> Asad

Thanks Asad for your explaination !

`LOCKDOWN_PCI_ACCESS` is used in the `integrity` mode of Lockdown and
should be used to protect against Kernel integrity tampering.

IIUC your issue, the access is read-only and thus, you should use one of
the existing `confidentiality` lockdown_reasons. You are free to add a
new one if none of the existing one covers your use case.

Best regards,

Nicolas