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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <mctt3nciphct26ogo5xugfigrwrz3tc3mq4qxludpmyfcfszlm@sfgv6c5lq3x4>
Date: Tue, 15 Jul 2025 23:29:12 -0400
From: Ivan Pravdin <ipravdin.official@...il.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
Cc: marcel@...tmann.org, johan.hedberg@...il.com, luiz.dentz@...il.com, 
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND] Bluetooth: vhci: Prevent use-after-free by
 removing debugfs files early

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@...il.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@...gen.mpg.de>

Thanks.

> 
> 
> Kind regards,
> 
> Paul

	Ivan Pravdin