1. Patchew
  2. linux
  3. View series

[PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early

Ivan Pravdin posted 1 patch 2 months, 3 weeks ago
There is a newer version of this series
drivers/bluetooth/hci_vhci.c | 57 ++++++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 16 deletions(-)
[PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early
Posted by Ivan Pravdin 2 months, 3 weeks ago 
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
Re: [PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early
Posted by Paul Menzel 2 months, 3 weeks ago 
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
Re: [PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by removing debugfs files early
Posted by Ivan Pravdin 2 months, 3 weeks ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.