RE: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition

Posted by Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) 2 years, 3 months ago

-----Original Message-----
From: Greg KH <gregkh@linuxfoundation.org> 
Sent: Friday, September 8, 2023 12:39 PM
To: Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) <deeratho@cisco.com>
Cc: stable@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition

> A: http://en.wikipedia.org/wiki/Top_post
> Q: Were do I find info about this thing called top-posting?
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?

> A: No.
> Q: Should I include quotations after my reply?


> http://daringfireball.net/2007/07/on_top

On Fri, Sep 08, 2023 at 06:54:06AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> Hi Greg,
> 
> This change is required to fix kernel CVE: CVE-2023-1989 which is 
> reported in v6.1 kernel version.

> Which change?

[Deepak]: I am referring below change. This below change is required to fix kernel CVE: CVE-2023-1989 which is reported in v6.1 kernel.

Subject: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition

From: Zheng Wang <zyytlz.wz@163.com>

[ Upstream commit 73f7b171b7c09139eb3c6a5677c200dc1be5f318 ]

In btsdio_probe, the data->work is bound with btsdio_work. It will be started in btsdio_send_frame.

If the btsdio_remove runs with a unfinished work, there may be a race condition that hdev is freed but used in btsdio_work. Fix it by canceling the work before do cleanup in btsdio_remove.

Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Deepak Rathore <deeratho@cisco.com>

diff --git a/drivers/bluetooth/btsdio.c b/drivers/bluetooth/btsdio.c index 795be33f2892..f19d31ee37ea 100644
--- a/drivers/bluetooth/btsdio.c
+++ b/drivers/bluetooth/btsdio.c
@@ -357,6 +357,7 @@ static void btsdio_remove(struct sdio_func *func)
 	if (!data)
 		return;
 
+	cancel_work_sync(&data->work);
 	hdev = data->hdev;
 
 	sdio_set_drvdata(func, NULL);
--
2.35.6


> It is fixed in upstream starting from v6.3 kernel version and required 
> to fix in 6.1 kernel version as well so we have backported this from
> v6.3 kernel version to v6.1 and I have sent this patch for review and 
> merging.

> Again, what commit are you referring to here.

> confused,

> greg k-h

[Deepak]: Sorry for the inconvenience that my message did not provide all the details.
The kernel CVE: CVE-2023-1989 is fixed in upstream with this commit: https://github.com/torvalds/linux/commit/73f7b171b7c09139eb3c6a5677c200dc1be5f318
Starting from v6.3 kernel and we have to fix this in 6.1 kernel as well, so we have backported this from v6.3 kernel version to v6.1 kernel.

Regards,
Deepak

Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition

Posted by Greg KH 2 years, 3 months ago

On Sat, Sep 09, 2023 at 08:49:52AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org> 
> Sent: Friday, September 8, 2023 12:39 PM
> To: Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) <deeratho@cisco.com>
> Cc: stable@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition
> 
> > A: http://en.wikipedia.org/wiki/Top_post
> > Q: Were do I find info about this thing called top-posting?
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > A: Top-posting.
> > Q: What is the most annoying thing in e-mail?
> 
> > A: No.
> > Q: Should I include quotations after my reply?
> 
> 
> > http://daringfireball.net/2007/07/on_top
> 
> On Fri, Sep 08, 2023 at 06:54:06AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> > Hi Greg,
> > 
> > This change is required to fix kernel CVE: CVE-2023-1989 which is 
> > reported in v6.1 kernel version.
> 
> > Which change?
> 
> [Deepak]: I am referring below change. This below change is required to fix kernel CVE: CVE-2023-1989 which is reported in v6.1 kernel.
> 
> Subject: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition
> 
> From: Zheng Wang <zyytlz.wz@163.com>
> 
> [ Upstream commit 73f7b171b7c09139eb3c6a5677c200dc1be5f318 ]

This commit is already in the 6.1.52 kernel release, why do you want it
included again?

confused,

greg k-h

RE: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition

Posted by Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) 2 years, 3 months ago

-----Original Message-----
From: Greg KH <gregkh@linuxfoundation.org> 
Sent: Saturday, September 9, 2023 5:17 PM
To: Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) <deeratho@cisco.com>
Cc: stable@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition

On Sat, Sep 09, 2023 at 08:49:52AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Friday, September 8, 2023 12:39 PM
> To: Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) 
> <deeratho@cisco.com>
> Cc: stable@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free 
> bug in btsdio_remove due to race condition
> 
> > A: http://en.wikipedia.org/wiki/Top_post
> > Q: Were do I find info about this thing called top-posting?
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > A: Top-posting.
> > Q: What is the most annoying thing in e-mail?
> 
> > A: No.
> > Q: Should I include quotations after my reply?
> 
> 
> > http://daringfireball.net/2007/07/on_top
> 
> On Fri, Sep 08, 2023 at 06:54:06AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> > Hi Greg,
> > 
> > This change is required to fix kernel CVE: CVE-2023-1989 which is 
> > reported in v6.1 kernel version.
> 
> > Which change?
> 
> [Deepak]: I am referring below change. This below change is required to fix kernel CVE: CVE-2023-1989 which is reported in v6.1 kernel.
> 
> Subject: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in 
> btsdio_remove due to race condition
> 
> From: Zheng Wang <zyytlz.wz@163.com>
> 
> [ Upstream commit 73f7b171b7c09139eb3c6a5677c200dc1be5f318 ]

> This commit is already in the 6.1.52 kernel release, why do you want it included again?

> confused,

> greg k-h

Hi Greg, Salvatore,

When I have submitted this patch for review, at that time, 6.1.52 was not released.

It will be good if you can share me guideline or details like how I can share CVE fix patch to upstream for review like what details I need to include in patch for review so from next time, we can save time in query discussion.

Regards,
Deepak

Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition

Posted by Greg KH 2 years, 3 months ago

On Sun, Sep 10, 2023 at 06:25:22AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org> 
> Sent: Saturday, September 9, 2023 5:17 PM
> To: Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) <deeratho@cisco.com>
> Cc: stable@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition
> 
> On Sat, Sep 09, 2023 at 08:49:52AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Friday, September 8, 2023 12:39 PM
> > To: Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) 
> > <deeratho@cisco.com>
> > Cc: stable@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free 
> > bug in btsdio_remove due to race condition
> > 
> > > A: http://en.wikipedia.org/wiki/Top_post
> > > Q: Were do I find info about this thing called top-posting?
> > > A: Because it messes up the order in which people normally read text.
> > > Q: Why is top-posting such a bad thing?
> > > A: Top-posting.
> > > Q: What is the most annoying thing in e-mail?
> > 
> > > A: No.
> > > Q: Should I include quotations after my reply?
> > 
> > 
> > > http://daringfireball.net/2007/07/on_top
> > 
> > On Fri, Sep 08, 2023 at 06:54:06AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> > > Hi Greg,
> > > 
> > > This change is required to fix kernel CVE: CVE-2023-1989 which is 
> > > reported in v6.1 kernel version.
> > 
> > > Which change?
> > 
> > [Deepak]: I am referring below change. This below change is required to fix kernel CVE: CVE-2023-1989 which is reported in v6.1 kernel.
> > 
> > Subject: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in 
> > btsdio_remove due to race condition
> > 
> > From: Zheng Wang <zyytlz.wz@163.com>
> > 
> > [ Upstream commit 73f7b171b7c09139eb3c6a5677c200dc1be5f318 ]
> 
> > This commit is already in the 6.1.52 kernel release, why do you want it included again?
> 
> > confused,
> 
> > greg k-h
> 
> Hi Greg, Salvatore,
> 
> When I have submitted this patch for review, at that time, 6.1.52 was not released.
> 
> It will be good if you can share me guideline or details like how I
> can share CVE fix patch to upstream for review like what details I
> need to include in patch for review so from next time, we can save
> time in query discussion.

Why does the random assignment of a CVE number mean anything should be
done differently than the normal process of getting a stable patch
merged?

You have read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

right?

That should cover it.

thanks,

greg k-h

Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition

Posted by Salvatore Bonaccorso 2 years, 3 months ago

On Sat, Sep 09, 2023 at 08:49:52AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org> 
> Sent: Friday, September 8, 2023 12:39 PM
> To: Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) <deeratho@cisco.com>
> Cc: stable@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition
> 
> > A: http://en.wikipedia.org/wiki/Top_post
> > Q: Were do I find info about this thing called top-posting?
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > A: Top-posting.
> > Q: What is the most annoying thing in e-mail?
> 
> > A: No.
> > Q: Should I include quotations after my reply?
> 
> 
> > http://daringfireball.net/2007/07/on_top
> 
> On Fri, Sep 08, 2023 at 06:54:06AM +0000, Deepak Rathore -X (deeratho - E-INFO CHIPS INC at Cisco) wrote:
> > Hi Greg,
> > 
> > This change is required to fix kernel CVE: CVE-2023-1989 which is 
> > reported in v6.1 kernel version.
> 
> > Which change?
> 
> [Deepak]: I am referring below change. This below change is required to fix kernel CVE: CVE-2023-1989 which is reported in v6.1 kernel.
> 
> Subject: [v6.1.52][PATCH] Bluetooth: btsdio: fix use after free bug in btsdio_remove due to race condition
> 
> From: Zheng Wang <zyytlz.wz@163.com>
> 
> [ Upstream commit 73f7b171b7c09139eb3c6a5677c200dc1be5f318 ]
> 
> In btsdio_probe, the data->work is bound with btsdio_work. It will be started in btsdio_send_frame.
> 
> If the btsdio_remove runs with a unfinished work, there may be a race condition that hdev is freed but used in btsdio_work. Fix it by canceling the work before do cleanup in btsdio_remove.
> 
> Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> Signed-off-by: Deepak Rathore <deeratho@cisco.com>
> 
> diff --git a/drivers/bluetooth/btsdio.c b/drivers/bluetooth/btsdio.c index 795be33f2892..f19d31ee37ea 100644
> --- a/drivers/bluetooth/btsdio.c
> +++ b/drivers/bluetooth/btsdio.c
> @@ -357,6 +357,7 @@ static void btsdio_remove(struct sdio_func *func)
>  	if (!data)
>  		return;
>  
> +	cancel_work_sync(&data->work);
>  	hdev = data->hdev;
>  
>  	sdio_set_drvdata(func, NULL);
> --
> 2.35.6
> 
> 
> > It is fixed in upstream starting from v6.3 kernel version and required 
> > to fix in 6.1 kernel version as well so we have backported this from
> > v6.3 kernel version to v6.1 and I have sent this patch for review and 
> > merging.
> 
> > Again, what commit are you referring to here.
> 
> > confused,
> 
> > greg k-h
> 
> [Deepak]: Sorry for the inconvenience that my message did not provide all the details.
> The kernel CVE: CVE-2023-1989 is fixed in upstream with this commit: https://github.com/torvalds/linux/commit/73f7b171b7c09139eb3c6a5677c200dc1be5f318
> Starting from v6.3 kernel and we have to fix this in 6.1 kernel as well, so we have backported this from v6.3 kernel version to v6.1 kernel.

This change was already backported to 6.1.y and released in v6.1.52?

It is commit 179c65828593aff1f444e15debd40a477cb23cf4 .

Regards,
Salvatore