Re: [REPORT] efi_secret: potential use-after-unmap via stale seq_file private after device removal

[Adding folks who were involved in the efi_secret driver]

I also think this should probably just be discussed on the mailing
list...

Will

(original report below)

On Fri, Apr 03, 2026 at 05:10:17PM +0800, chichina c wrote:
>    Hello Linux kernel security team,
> 
>    I would like to report a potential security issue in
>    drivers/virt/coco/efi_secret/efi_secret.c.
> 
>    The issue appears to be a stale pointer / use-after-unmap in the
>    securityfs read path after device removal.
> 
>    In efi_secret_securityfs_setup(), each securityfs file is created with
>    its private data pointing directly to a struct secret_entry inside the
>    mapped EFI secret area:
> 
>  e = (struct secret_entry *)ptr;
>  securityfs_create_file(guid_str, 0440, dir, (void *)e,
>                         &efi_secret_bin_file_fops);
> 
>    The file uses DEFINE_SHOW_ATTRIBUTE(efi_secret_bin_file), so open()
>    eventually calls:
> 
>  single_open(file, efi_secret_bin_file_show, inode->i_private);
> 
>    This means an already-open file descriptor keeps the secret_entry pointer
>    in seq_file->private.
> 
>    Later, efi_secret_bin_file_show() uses that saved pointer:
> 
>  struct secret_entry *e = file->private;
>  if (e)
>          seq_write(file, e->data, secret_entry_data_len(e));
> 
>    The problem is that the lifetime of this pointer is tied to the EFI
>    secret area mapping. The secret_entry object is not separately allocated;
>    it lives inside s->secret_data.
> 
>    On device removal, the driver does:
> 
>  efi_secret_securityfs_teardown(dev);
>  efi_secret_unmap_area();
> 
>    and efi_secret_unmap_area() calls:
> 
>  iounmap(s->secret_data);
> 
>    At that point, an already-open file descriptor may still hold the old
>    secret_entry pointer in seq_file->private. A later read on that old fd
>    appears able to reach efi_secret_bin_file_show() and dereference
>    e->len / e->data after the backing mapping has been removed.
> 
>    My understanding is that securityfs_remove() tears down the dentry/inode
>    tree, but it does not invalidate seq_file->private for file descriptors
>    that were already opened before removal. Also, the show path uses
>    single_release() only, with no extra pinning or refcounting of the
>    underlying secret_entry object.
> 
>    Expected trigger sequence:
> 
>     1. The platform exposes EFI coco secrets and the driver creates
>        /sys/kernel/security/secrets/coco/.
>     2. A process opens one of the secret files and keeps the fd open.
>     3. The device is removed or unbound, so efi_secret_remove() runs and
>        unmaps s->secret_data.
>     4. The process reads again from the already-open fd.
>    probe
>     └─ efi_secret_map_area
>         └─ s->secret_data = ioremap_encrypted(...)
> 
>    probe
>     └─ efi_secret_securityfs_setup
>         ├─ e = (struct secret_entry *)ptr
>         └─ securityfs_create_file(..., data=e, ...)
> 
>    open(fd)
>     └─ efi_secret_bin_file_open
>         └─ single_open(file, show, inode->i_private)
>             └─ seq_file->private = e
> 
>    read(fd)
>     └─ seq_read
>         └─ efi_secret_bin_file_show
>             └─ e = seq_file->private
>             └─ read e->len / e->data
> 
>    remove
>     └─ efi_secret_remove
>         ├─ securityfs_remove(...)
>         └─ iounmap(s->secret_data)
> 
>    read(old fd again)
>     └─ seq_read
>         └─ efi_secret_bin_file_show
>             └─ use old seq_file->private = stale e
>             └─ access unmapped memory
> 
>    The expected result is a stale dereference of a secret_entry pointer
>    whose backing memory has already been unmapped. At minimum this looks
>    like a local kernel crash / DoS issue.
> 
>    This report is currently based on source analysis.
> 
>    Environment:
>    Kernel: [v7.0-rc6]
> 
>    A possible fix would be one of the following:
> 
>      • avoid storing raw pointers into the mapped secret area in
>        seq_file->private;
>      • use a separately allocated refcounted object whose lifetime is not
>        tied directly to the ioremap mapping;
>      • delay iounmap(s->secret_data) until all open readers are gone;
>      • explicitly invalidate or drain already-open readers before unmapping.
>        |
> 
>    If this is confirmed to be a real issue and fixed, I would appreciate it
>    if my reporter information could be included in the report/changelog.
> 
>    Best regards,
> 
>    chichi
> 
>    [1][email protected]
> 
> References
> 
>    Visible links
>    1. mailto:[email protected]