* [PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early
@ 2025-07-16 0:42 Ivan Pravdin
2025-07-16 3:26 ` Paul Menzel
0 siblings, 1 reply; 3+ messages in thread
From: Ivan Pravdin @ 2025-07-16 0:42 UTC (permalink / raw)
To: marcel, johan.hedberg, luiz.dentz, linux-bluetooth, linux-kernel
Cc: Ivan Pravdin
Move the creation of debugfs files into a dedicated function, and ensure
they are explicitly removed during vhci_release(), before associated
data structures are freed.
Previously, debugfs files such as "force_suspend", "force_wakeup", and
others were created under hdev->debugfs but not removed in
vhci_release(). Since vhci_release() frees the backing vhci_data
structure, any access to these files after release would result in
use-after-free errors.
Although hdev->debugfs is later freed in hci_release_dev(), user can
access files after vhci_data is freed but before hdev->debugfs is
released.
Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.com>
Fixes: ab4e4380d4e1 ("Bluetooth: Add vhci devcoredump support")
---
Resending because previous patch got archived [1].
[1] https://patchwork.kernel.org/project/bluetooth/patch/20250614235317.304705-1-ipravdin.official@gmail.com/
drivers/bluetooth/hci_vhci.c | 57 ++++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 16 deletions(-)
diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index f7d8c3c00655..2fef08254d78 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -380,6 +380,28 @@ static const struct file_operations force_devcoredump_fops = {
.write = force_devcd_write,
};
+static void vhci_debugfs_init(struct vhci_data *data)
+{
+ struct hci_dev *hdev = data->hdev;
+
+ debugfs_create_file("force_suspend", 0644, hdev->debugfs, data,
+ &force_suspend_fops);
+
+ debugfs_create_file("force_wakeup", 0644, hdev->debugfs, data,
+ &force_wakeup_fops);
+
+ if (IS_ENABLED(CONFIG_BT_MSFTEXT))
+ debugfs_create_file("msft_opcode", 0644, hdev->debugfs, data,
+ &msft_opcode_fops);
+
+ if (IS_ENABLED(CONFIG_BT_AOSPEXT))
+ debugfs_create_file("aosp_capable", 0644, hdev->debugfs, data,
+ &aosp_capable_fops);
+
+ debugfs_create_file("force_devcoredump", 0644, hdev->debugfs, data,
+ &force_devcoredump_fops);
+}
+
static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
{
struct hci_dev *hdev;
@@ -434,22 +456,8 @@ static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
return -EBUSY;
}
- debugfs_create_file("force_suspend", 0644, hdev->debugfs, data,
- &force_suspend_fops);
-
- debugfs_create_file("force_wakeup", 0644, hdev->debugfs, data,
- &force_wakeup_fops);
-
- if (IS_ENABLED(CONFIG_BT_MSFTEXT))
- debugfs_create_file("msft_opcode", 0644, hdev->debugfs, data,
- &msft_opcode_fops);
-
- if (IS_ENABLED(CONFIG_BT_AOSPEXT))
- debugfs_create_file("aosp_capable", 0644, hdev->debugfs, data,
- &aosp_capable_fops);
-
- debugfs_create_file("force_devcoredump", 0644, hdev->debugfs, data,
- &force_devcoredump_fops);
+ if (!IS_ERR_OR_NULL(hdev->debugfs))
+ vhci_debugfs_init(data);
hci_skb_pkt_type(skb) = HCI_VENDOR_PKT;
@@ -651,6 +659,21 @@ static int vhci_open(struct inode *inode, struct file *file)
return 0;
}
+static void vhci_debugfs_remove(struct hci_dev *hdev)
+{
+ debugfs_lookup_and_remove("force_suspend", hdev->debugfs);
+
+ debugfs_lookup_and_remove("force_wakeup", hdev->debugfs);
+
+ if (IS_ENABLED(CONFIG_BT_MSFTEXT))
+ debugfs_lookup_and_remove("msft_opcode", hdev->debugfs);
+
+ if (IS_ENABLED(CONFIG_BT_AOSPEXT))
+ debugfs_lookup_and_remove("aosp_capable", hdev->debugfs);
+
+ debugfs_lookup_and_remove("force_devcoredump", hdev->debugfs);
+}
+
static int vhci_release(struct inode *inode, struct file *file)
{
struct vhci_data *data = file->private_data;
@@ -662,6 +685,8 @@ static int vhci_release(struct inode *inode, struct file *file)
hdev = data->hdev;
if (hdev) {
+ if (!IS_ERR_OR_NULL(hdev->debugfs))
+ vhci_debugfs_remove(hdev);
hci_unregister_dev(hdev);
hci_free_dev(hdev);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early
2025-07-16 0:42 [PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early Ivan Pravdin
@ 2025-07-16 3:26 ` Paul Menzel
2025-07-16 3:29 ` Ivan Pravdin
0 siblings, 1 reply; 3+ messages in thread
From: Paul Menzel @ 2025-07-16 3:26 UTC (permalink / raw)
To: Ivan Pravdin
Cc: marcel, johan.hedberg, luiz.dentz, linux-bluetooth, linux-kernel
Dear Ivan,
Thank you for your patch.
Am 16.07.25 um 02:42 schrieb Ivan Pravdin:
> Move the creation of debugfs files into a dedicated function, and ensure
> they are explicitly removed during vhci_release(), before associated
> data structures are freed.
>
> Previously, debugfs files such as "force_suspend", "force_wakeup", and
> others were created under hdev->debugfs but not removed in
> vhci_release(). Since vhci_release() frees the backing vhci_data
> structure, any access to these files after release would result in
> use-after-free errors.
>
> Although hdev->debugfs is later freed in hci_release_dev(), user can
> access files after vhci_data is freed but before hdev->debugfs is
> released.
>
> Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.com>
> Fixes: ab4e4380d4e1 ("Bluetooth: Add vhci devcoredump support")
Not important, but I’d put the Signed-off-by: tag last.
> ---
> Resending because previous patch got archived [1].
>
> [1] https://patchwork.kernel.org/project/bluetooth/patch/20250614235317.304705-1-ipravdin.official@gmail.com/
>
> drivers/bluetooth/hci_vhci.c | 57 ++++++++++++++++++++++++++----------
> 1 file changed, 41 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> index f7d8c3c00655..2fef08254d78 100644
> --- a/drivers/bluetooth/hci_vhci.c
> +++ b/drivers/bluetooth/hci_vhci.c
> @@ -380,6 +380,28 @@ static const struct file_operations force_devcoredump_fops = {
> .write = force_devcd_write,
> };
>
> +static void vhci_debugfs_init(struct vhci_data *data)
> +{
> + struct hci_dev *hdev = data->hdev;
> +
> + debugfs_create_file("force_suspend", 0644, hdev->debugfs, data,
> + &force_suspend_fops);
> +
> + debugfs_create_file("force_wakeup", 0644, hdev->debugfs, data,
> + &force_wakeup_fops);
> +
> + if (IS_ENABLED(CONFIG_BT_MSFTEXT))
> + debugfs_create_file("msft_opcode", 0644, hdev->debugfs, data,
> + &msft_opcode_fops);
> +
> + if (IS_ENABLED(CONFIG_BT_AOSPEXT))
> + debugfs_create_file("aosp_capable", 0644, hdev->debugfs, data,
> + &aosp_capable_fops);
> +
> + debugfs_create_file("force_devcoredump", 0644, hdev->debugfs, data,
> + &force_devcoredump_fops);
> +}
> +
> static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
> {
> struct hci_dev *hdev;
> @@ -434,22 +456,8 @@ static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
> return -EBUSY;
> }
>
> - debugfs_create_file("force_suspend", 0644, hdev->debugfs, data,
> - &force_suspend_fops);
> -
> - debugfs_create_file("force_wakeup", 0644, hdev->debugfs, data,
> - &force_wakeup_fops);
> -
> - if (IS_ENABLED(CONFIG_BT_MSFTEXT))
> - debugfs_create_file("msft_opcode", 0644, hdev->debugfs, data,
> - &msft_opcode_fops);
> -
> - if (IS_ENABLED(CONFIG_BT_AOSPEXT))
> - debugfs_create_file("aosp_capable", 0644, hdev->debugfs, data,
> - &aosp_capable_fops);
> -
> - debugfs_create_file("force_devcoredump", 0644, hdev->debugfs, data,
> - &force_devcoredump_fops);
> + if (!IS_ERR_OR_NULL(hdev->debugfs))
> + vhci_debugfs_init(data);
>
> hci_skb_pkt_type(skb) = HCI_VENDOR_PKT;
>
> @@ -651,6 +659,21 @@ static int vhci_open(struct inode *inode, struct file *file)
> return 0;
> }
>
> +static void vhci_debugfs_remove(struct hci_dev *hdev)
> +{
> + debugfs_lookup_and_remove("force_suspend", hdev->debugfs);
> +
> + debugfs_lookup_and_remove("force_wakeup", hdev->debugfs);
> +
> + if (IS_ENABLED(CONFIG_BT_MSFTEXT))
> + debugfs_lookup_and_remove("msft_opcode", hdev->debugfs);
> +
> + if (IS_ENABLED(CONFIG_BT_AOSPEXT))
> + debugfs_lookup_and_remove("aosp_capable", hdev->debugfs);
> +
> + debugfs_lookup_and_remove("force_devcoredump", hdev->debugfs);
> +}
> +
> static int vhci_release(struct inode *inode, struct file *file)
> {
> struct vhci_data *data = file->private_data;
> @@ -662,6 +685,8 @@ static int vhci_release(struct inode *inode, struct file *file)
> hdev = data->hdev;
>
> if (hdev) {
> + if (!IS_ERR_OR_NULL(hdev->debugfs))
> + vhci_debugfs_remove(hdev);
> hci_unregister_dev(hdev);
> hci_free_dev(hdev);
> }
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early
2025-07-16 3:26 ` Paul Menzel
@ 2025-07-16 3:29 ` Ivan Pravdin
0 siblings, 0 replies; 3+ messages in thread
From: Ivan Pravdin @ 2025-07-16 3:29 UTC (permalink / raw)
To: Paul Menzel
Cc: marcel, johan.hedberg, luiz.dentz, linux-bluetooth, linux-kernel
On Wed, Jul 16, 2025 at 05:26:05AM GMT, Paul Menzel wrote:
> Dear Ivan,
>
>
> Thank you for your patch.
>
> Am 16.07.25 um 02:42 schrieb Ivan Pravdin:
> > Move the creation of debugfs files into a dedicated function, and ensure
> > they are explicitly removed during vhci_release(), before associated
> > data structures are freed.
> >
> > Previously, debugfs files such as "force_suspend", "force_wakeup", and
> > others were created under hdev->debugfs but not removed in
> > vhci_release(). Since vhci_release() frees the backing vhci_data
> > structure, any access to these files after release would result in
> > use-after-free errors.
> >
> > Although hdev->debugfs is later freed in hci_release_dev(), user can
> > access files after vhci_data is freed but before hdev->debugfs is
> > released.
> >
> > Signed-off-by: Ivan Pravdin <ipravdin.official@gmail.com>
> > Fixes: ab4e4380d4e1 ("Bluetooth: Add vhci devcoredump support")
>
> Not important, but I’d put the Signed-off-by: tag last.
Not sure how I missed that, sorry!
>
> > ---
> > Resending because previous patch got archived [1].
> >
> > [1] https://patchwork.kernel.org/project/bluetooth/patch/20250614235317.304705-1-ipravdin.official@gmail.com/
> >
> > drivers/bluetooth/hci_vhci.c | 57 ++++++++++++++++++++++++++----------
> > 1 file changed, 41 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> > index f7d8c3c00655..2fef08254d78 100644
> > --- a/drivers/bluetooth/hci_vhci.c
> > +++ b/drivers/bluetooth/hci_vhci.c
> > @@ -380,6 +380,28 @@ static const struct file_operations force_devcoredump_fops = {
> > .write = force_devcd_write,
> > };
> > +static void vhci_debugfs_init(struct vhci_data *data)
> > +{
> > + struct hci_dev *hdev = data->hdev;
> > +
> > + debugfs_create_file("force_suspend", 0644, hdev->debugfs, data,
> > + &force_suspend_fops);
> > +
> > + debugfs_create_file("force_wakeup", 0644, hdev->debugfs, data,
> > + &force_wakeup_fops);
> > +
> > + if (IS_ENABLED(CONFIG_BT_MSFTEXT))
> > + debugfs_create_file("msft_opcode", 0644, hdev->debugfs, data,
> > + &msft_opcode_fops);
> > +
> > + if (IS_ENABLED(CONFIG_BT_AOSPEXT))
> > + debugfs_create_file("aosp_capable", 0644, hdev->debugfs, data,
> > + &aosp_capable_fops);
> > +
> > + debugfs_create_file("force_devcoredump", 0644, hdev->debugfs, data,
> > + &force_devcoredump_fops);
> > +}
> > +
> > static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
> > {
> > struct hci_dev *hdev;
> > @@ -434,22 +456,8 @@ static int __vhci_create_device(struct vhci_data *data, __u8 opcode)
> > return -EBUSY;
> > }
> > - debugfs_create_file("force_suspend", 0644, hdev->debugfs, data,
> > - &force_suspend_fops);
> > -
> > - debugfs_create_file("force_wakeup", 0644, hdev->debugfs, data,
> > - &force_wakeup_fops);
> > -
> > - if (IS_ENABLED(CONFIG_BT_MSFTEXT))
> > - debugfs_create_file("msft_opcode", 0644, hdev->debugfs, data,
> > - &msft_opcode_fops);
> > -
> > - if (IS_ENABLED(CONFIG_BT_AOSPEXT))
> > - debugfs_create_file("aosp_capable", 0644, hdev->debugfs, data,
> > - &aosp_capable_fops);
> > -
> > - debugfs_create_file("force_devcoredump", 0644, hdev->debugfs, data,
> > - &force_devcoredump_fops);
> > + if (!IS_ERR_OR_NULL(hdev->debugfs))
> > + vhci_debugfs_init(data);
> > hci_skb_pkt_type(skb) = HCI_VENDOR_PKT;
> > @@ -651,6 +659,21 @@ static int vhci_open(struct inode *inode, struct file *file)
> > return 0;
> > }
> > +static void vhci_debugfs_remove(struct hci_dev *hdev)
> > +{
> > + debugfs_lookup_and_remove("force_suspend", hdev->debugfs);
> > +
> > + debugfs_lookup_and_remove("force_wakeup", hdev->debugfs);
> > +
> > + if (IS_ENABLED(CONFIG_BT_MSFTEXT))
> > + debugfs_lookup_and_remove("msft_opcode", hdev->debugfs);
> > +
> > + if (IS_ENABLED(CONFIG_BT_AOSPEXT))
> > + debugfs_lookup_and_remove("aosp_capable", hdev->debugfs);
> > +
> > + debugfs_lookup_and_remove("force_devcoredump", hdev->debugfs);
> > +}
> > +
> > static int vhci_release(struct inode *inode, struct file *file)
> > {
> > struct vhci_data *data = file->private_data;
> > @@ -662,6 +685,8 @@ static int vhci_release(struct inode *inode, struct file *file)
> > hdev = data->hdev;
> > if (hdev) {
> > + if (!IS_ERR_OR_NULL(hdev->debugfs))
> > + vhci_debugfs_remove(hdev);
> > hci_unregister_dev(hdev);
> > hci_free_dev(hdev);
> > }
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Thanks.
>
>
> Kind regards,
>
> Paul
Ivan Pravdin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-16 3:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-16 0:42 [PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early Ivan Pravdin
2025-07-16 3:26 ` Paul Menzel
2025-07-16 3:29 ` Ivan Pravdin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).