mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: "Anh Khoa Vũ Nguyễn" <khoavna.tin.2225@gmail.com>
Cc: jirislaby@kernel.org, arnd@arndb.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] misc: phantom: fix open file UAF after device removal
Date: Thu, 1 Oct 2026 13:54:29 +0200	[thread overview]
Message-ID: <2026100113-tribute-explore-922b@gregkh> (raw)
In-Reply-To: <CAK5ebCne_H-p9yaastoJnMk7k8SXtiaRohZruXN3xZGvsRa8zg@mail.gmail.com>

On Tue, Sep 01, 2026 at 05:30:08PM +0700, Anh Khoa Vũ Nguyễn wrote:
> Hi Greg,
> 
> Thanks for the review.
> 
> This was found by source audit, not by a runtime crash report.
> 
> I was checking char drivers for the pattern where:
> - ->open() stores a raw device pointer in file->private_data,
> - ->remove() calls cdev_del() and frees the enclosing object, and
> - already-open file descriptors can still reach
> ->ioctl()/->poll()/->release()
>   after cdev_del() returns.
> 
> phantom.c seemed to match that pattern:
> - phantom_open() stores struct phantom_device * in file->private_data
> - phantom_remove() calls cdev_del() and then kfree(pht)
> - phantom_ioctl(), phantom_poll(), and phantom_release() all dereference
>   file->private_data
> - __fput() calls ->release() before cdev_put(), so the enclosing object
>   still needs to remain valid for an already-open file
> 
> The remove path I had in mind was manual driver unbind while the device node
> is still open, e.g.
> 
>   echo -n 0000:BB:DD.F > /sys/bus/pci/drivers/phantom/unbind
> 
> That should call phantom_remove() even if a userspace process still has
> /dev/phantomX open. The open file pins the module via .owner, but it does
> not pin struct phantom_device itself.
> 
> I should have been clearer in the changelog that this was an audit-found
> lifetime issue and that the trigger I had in mind was sysfs unbind.
> 
> As for testing, I only compile-tested the patch and ran checkpatch. I do not
> have Phantom hardware, so I have not reproduced this on a live system.
> 
> Also, yes, I can simplify the locking change and use a guard/scoped_guard
> helper in a v2 if the issue is worth pursuing.
> 
> If this is considered too theoretical without runtime confirmation on actual
> hardware, I understand and can drop the patch.

Please test it properly and send an update if you wish for it to be
accepted.

thanks,

greg k-h

      parent reply	other threads:[~2026-10-01 12:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 15:13 Vu Nguyen Anh Khoa
2026-09-01  4:41 ` Greg KH
     [not found]   ` <CAK5ebCne_H-p9yaastoJnMk7k8SXtiaRohZruXN3xZGvsRa8zg@mail.gmail.com>
2026-09-01 15:56     ` Greg KH
2026-10-01 11:54     ` Greg KH [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026100113-tribute-explore-922b@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=arnd@arndb.de \
    --cc=jirislaby@kernel.org \
    --cc=khoavna.tin.2225@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®