archive mirror
 help / color / mirror / Atom feed
From: Anant Thazhemadam <>
To: Hans de Goede <>
	Johan Hedberg <>,
	Marcel Holtmann <>,,,
Subject: Re: [Linux-kernel-mentees] [PATCH v3] bluetooth: hci_h5: fix memory leak in h5_close
Date: Tue, 6 Oct 2020 08:14:45 +0530	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 05-10-2020 14:48, Hans de Goede wrote:
> To fully fix the memleak you also need to add a kfree_skb(h5->rx_skb);
> call to the end of h5_serdev_remove(), because in the hu->serdev case
> that is where the h5 struct will be free-ed (it is free-ed after that
> function exits).

Hi Hans,

I'm not entirely convinced that it might be entirely the best idea to do

* The bug detected by syzbot only provides us with reproducer and
information about this bug (which gets triggered when !hu->serdev).
Even if like you said, there might be a memory leak unattended to when
hu->serdev exists, then this might not necessarily be the right place to fix

* From what I can see, all the drivers that have modified to provide serdev
support have different close() mechanisms.
However, one thing they do have in common (in this context) is that their
respective serdev_remove() function simply calls hci_uart_unregister_device()
to unregister the device.
It is primarily for this reason that I feel adding a kfree_skb() call at the end
of h5_serdev_remove() might not exactly be the best way we could solve this
(and since this hasn't been picked up by syzbot yet, there's no way to know if
this just fixes things or ends up causing unforeseen complications).

Alternatively, wouldn't freeing h5->rx_skb and assigning it to NULL, for both
hu->serdev and !hu->serdev cases within h5_close() itself be a better
approach? I've also taken the liberty of testing a patch that does this, and it
seems to work correctly too. :)

But then again, I'm not exactly an authority on how this works.

Linux-kernel-mentees mailing list

  reply	other threads:[~2020-10-06  2:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-01 19:43 [Linux-kernel-mentees] [PATCH v2] bluetooth: hci_h5: close serdev device and free hu in h5_close Anant Thazhemadam
2020-10-02 10:22 ` Hans de Goede
2020-10-02 10:55   ` Anant Thazhemadam
2020-10-03 22:07   ` Anant Thazhemadam
2020-10-04  5:17 ` [Linux-kernel-mentees] [PATCH v3] bluetooth: hci_h5: fix memory leak " Anant Thazhemadam
2020-10-05  9:18   ` Hans de Goede
2020-10-06  2:44     ` Anant Thazhemadam [this message]
2020-10-06  6:30       ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).