From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A40852D876B for ; Wed, 24 Jun 2026 10:56:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782298588; cv=none; b=Q2QW8rjj/Kx1bBbNLNvZ2KOOYxAH+kaORaZo/rWTDOuLO3dtLkrj2cveCFm049OyeD7tZbFFGkBOoBQBKTacYmO/MtsMMHSzWOMu3X+86ryhcXAC8zPqJYAIftkeqIpZ/NjHajAR1PwBihU9mJHPUdALQzMdYma5L6VzTVAgZi4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782298588; c=relaxed/simple; bh=OAskSojY9S9GzYSrZYLXDAReXtrR+BnRjCU+yWxpozY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SlKxZyO1nwXkpUl2rqzaWvYF/y9jf60DHUEr/qqoiFvY4t1JH/hmqGNLAhDOOAKfVMaZO8ThiGHH6pIPEbQg7iPcfNkCkXtEl2t3MRX/IT6Vw6kdqIwMess+EFGSHKPz1QHOqk0ZXoowYMfxFwyqnlsSewnc8117hpPcujJfy5g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=o8ZOZqM8; arc=none smtp.client-ip=209.85.208.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="o8ZOZqM8" Received: by mail-ed1-f48.google.com with SMTP id 4fb4d7f45d1cf-6974ef0c3b1so1314115a12.1 for ; Wed, 24 Jun 2026 03:56:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782298585; x=1782903385; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=wVpUsPMcOUp7aStO4/HxyiJ7/oBFz2LYmxiU2GOTN6E=; b=o8ZOZqM8yDNUIzsT1xgrKYsFpdCEV3AEUZWzxfMBIYg7cQun8KkVFzFLza5IVsXUo4 a1/6ArEMuiDf+rCm5yKbQ/kXWx6ZlNDd9ajtEItwOs4nj6whYfnPj1EVRzwbbL8b8CoC YsEI3JDhqPyFPImO8QU1TqEahjU5bu8QB1Cfg069iiU9TbbBkQLzHt78n8Qxp96YFN0F ZNyD8uNRuVERP73BKnCCUpuPf4DIEnyh2pcTDX7agTbx72B/fR9qy6Uy3AfbdWLPeXTE X5LwOFKPCXM5VYEKkrFY1sEGNNgYV1LQ66KHR+ncLzlPzYkboKa3dsz+WabLEZeDTj9C lmsw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782298585; x=1782903385; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wVpUsPMcOUp7aStO4/HxyiJ7/oBFz2LYmxiU2GOTN6E=; b=eR+zPjjzJoYhMKAEhfJM5nvKUDTRichn9k3yGASoGDf1GIPi3dRkVNqbsODkRaKVs/ ViVbaMJhSCtDoLhCWZVH9EXbi2WgX6oN2Uzamsl5YbvUeb6NMdqe2NVMtHoACs/1yebR D5TS4Cog2HiFc+jS8Z54fVzCLSdj6TafMW5ZEqcCtjOg+7YBtWaQzdBvZy/2rdoRokvo 9ahhDQli3hW7nmMSWnAGTx5ONKuQR/4MTOdS1rok/FXhBaEn0uPkWHuazpH1L78h1eSv /QPXNYDDVlQuBw4ecp3Bw7lcHG0g31zexp1kyYDO4dbajciMu/xf+xPu72cqr7zF4ayu GK7Q== X-Forwarded-Encrypted: i=1; AHgh+RrvRl6is4yOzBuRnDXr/LiL7Wn9f77obC82K1iYxkSfUstjfPCsv3tZ9FORvHBrwUqXnskwJnuBj2kcV+0=@vger.kernel.org X-Gm-Message-State: AOJu0YwFEgLELJEmtj9mxuX44c05sH56HSM+q8eoVHSA0eZsuc35vhsR TXp5trxiqB5W5Uuq6albVjpK37oXBVtmkI7l21jB3JniKYXZACUz/OcL X-Gm-Gg: AfdE7cmQR9NH+LikjbpMHVChxLnAZSKgb4dy6W01OvWU63k6IyQQEiCSB1Lm240G9kV OKl+IYTWz82+VHvm7aW9pZ8FKPsYxoIJBuVzDaa1dyvcaDYjmOFYOEcC3Z6AEigGqgjsXZoPLC4 S/CwD2JLOTj7r6tje5meWyIfSD5gQlGFIXSlkuTCDU4mq27QmUttQSN5xWCTlt3bplrGe11fjH5 u04fwBAKFCc0OSK5rdT4cD3qZ4e4cHZ7wRfQCDPI4VkPljXMgmDX/2OdvXKLSUO4TQPfWnV7P4h tng+ITfdmx6Dqrmyq4XElRZt9OEsnfLZ4FRtK9iwRU7rHUkd5iblVeffphBodtg7YXuzOPSuXAN GC5Dqdm1mdkhy9h7/QebkKEENVzc65MSaV0NlFEXt28hjzE3QKGQvkYnn5+nIff1Vcvc9QGl1aB 9EPFjcIn1y X-Received: by 2002:a05:6402:3218:b0:697:b10a:35ce with SMTP id 4fb4d7f45d1cf-697dba620e2mr3554762a12.1.1782298584851; Wed, 24 Jun 2026 03:56:24 -0700 (PDT) Received: from localhost ([196.207.164.177]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-697f3ac5df0sm1005336a12.1.2026.06.24.03.56.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Jun 2026 03:56:24 -0700 (PDT) Date: Wed, 24 Jun 2026 13:56:20 +0300 From: Dan Carpenter To: Haoxiang Li Cc: marcel@holtmann.org, luiz.dentz@gmail.com, yangyingliang@huawei.com, mst@redhat.com, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] Bluetooth: virtio_bt: unregister HCI device on open failure Message-ID: References: <20260624084333.2885144-1-haoxiang_li2024@163.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260624084333.2885144-1-haoxiang_li2024@163.com> On Wed, Jun 24, 2026 at 04:43:33PM +0800, Haoxiang Li wrote: > virtbt_probe() registers the HCI device before calling > virtbt_open_vdev(). If opening the virtio Bluetooth > device fails, the error path frees the HCI device without > unregistering it. > > Fixes: dc65b4b0f90a ("Bluetooth: virtio_bt: fix device removal") > Cc: stable@vger.kernel.org > Signed-off-by: Haoxiang Li > --- > drivers/bluetooth/virtio_bt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c > index 140ab55c9fc5..bf6827431bb8 100644 > --- a/drivers/bluetooth/virtio_bt.c > +++ b/drivers/bluetooth/virtio_bt.c > @@ -397,6 +397,7 @@ static int virtbt_probe(struct virtio_device *vdev) > return 0; > > open_failed: > + hci_unregister_dev(hdev); > hci_free_dev(hdev); > failed: > vdev->config->del_vqs(vdev); I have written a blog about how to write error handling. https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/ Originally this code using One Err style error handling where every error path just did "goto fail". It's also using ComeFrom label names which don't say what the goto does only where the goto is... Ideally if hci_register_dev() failed it would use the unwind ladder to clean up it instead calls hci_free_dev() and then goto fail. The beauty of writing a normal kernel style unwind ladder is that it writes the cleanup function automatically... Let's look at the cleanup function here. 406 static void virtbt_remove(struct virtio_device *vdev) 407 { 408 struct virtio_bluetooth *vbt = vdev->priv; 409 struct hci_dev *hdev = vbt->hdev; 410 411 hci_unregister_dev(hdev); 412 virtio_reset_device(vdev); 413 virtbt_close_vdev(vbt); I'm really uncomfortable with having the hci_unregister_dev() before the close. Potential use after free? 414 415 hci_free_dev(hdev); 416 vbt->hdev = NULL; 417 418 vdev->config->del_vqs(vdev); 419 kfree(vbt); The probe function should free "vbt" but it doesn't so that's another leak. 420 } So this fix is fine but it's also only a partial fix. regards, dan carpenter