Received-SPF: pass (google.com: domain of linux-kernel+bounces-146241-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 16 Apr 2024 06:49:43 +0200
From: Greg KH <gregkh@linuxfoundation.org>
To: sicong <congei42@163.com>
Cc: johan@kernel.org, elder@kernel.org, greybus-dev@lists.linaro.org,
	linux-kernel@vger.kernel.org
Subject: Re: Bug report: greybus/interface.c: use-after-free bug in
 gb_interface_release due to race condition
Message-ID: <2024041624-oxford-sabbath-9f6a@gregkh>
References: <280ee5e5.4936.18ee4d93bae.Coremail.congei42@163.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <280ee5e5.4936.18ee4d93bae.Coremail.congei42@163.com>

On Tue, Apr 16, 2024 at 11:00:25AM +0800, sicong wrote:
> greybus/interface.c: use-after-free bug in gb_interface_release due to
> race condition.
> 
> In gb_interface_create, &intf->mode_switch_completion is bound with 
> gb_interface_mode_switch_work. Then it will be started by 
> gb_interface_request_mode_switch. Here is the code.
> if (!queue_work(system_long_wq, &intf->mode_switch_work)) {
> 	...
> }
> 
> If we call gb_interface_release to make cleanup, there may be an 
> unfinished work. This function will call kfree to free the object 
> "intf". However, if gb_interface_mode_switch_work is scheduled to 
> run after kfree, it may cause use-after-free error as 
> gb_interface_mode_switch_work will use the object "intf". 
> The possible execution flow that may lead to the issue is as follows:
> 
> CPU0                            CPU1
> 
>                             |   gb_interface_create
>                             |   gb_interface_request_mode_switch
> gb_interface_release        |
> kfree(intf) (free)          |
>                             |   gb_interface_mode_switch_work
>                             |   mutex_lock(&intf->mutex) (use)   
> 
> This bug may be fixed by adding the following code before kfree.
> cancel_work_sync(&intf->mode_switch_work);

Wonderful, please submit a patch with this information and we will be
glad to review it.

thanks,

greg k-h