All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: jouni.hogander@unikie.com
Cc: linux-kernel@vger.kernel.org,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Lukas Bulwahn <lukas.bulwahn@gmail.com>
Subject: Re: [PATCH] drivers/base: Fix memory leak in error paths
Date: Fri, 15 Nov 2019 11:26:03 +0800	[thread overview]
Message-ID: <20191115032603.GG793701@kroah.com> (raw)
In-Reply-To: <20191114121840.5585-1-jouni.hogander@unikie.com>

On Thu, Nov 14, 2019 at 02:18:40PM +0200, jouni.hogander@unikie.com wrote:
> From: Jouni Hogander <jouni.hogander@unikie.com>
> 
> Currently error paths are using device_del to clean-up preparations
> done by device_add. This is causing memory leak as free of dev->p
> allocated in device_add is freed in device_release. This is fixed by
> moving freeing dev->p to counterpart of device_add i.e. device_del.

Are you sure that is safe?  The device can still be "alive" after
device_del() is called.  The only place you know that it should be freed
is in the release callback.

> This memory leak was reported by Syzkaller:
> 
> BUG: memory leak unreferenced object 0xffff8880675ca008 (size 256):
>   comm "netdev_register", pid 281, jiffies 4294696663 (age 6.808s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>   backtrace:
>     [<0000000058ca4711>] kmem_cache_alloc_trace+0x167/0x280
>     [<000000002340019b>] device_add+0x882/0x1750
>     [<000000001d588c3a>] netdev_register_kobject+0x128/0x380
>     [<0000000011ef5535>] register_netdevice+0xa1b/0xf00
>     [<000000007fcf1c99>] __tun_chr_ioctl+0x20d5/0x3dd0
>     [<000000006a5b7b2b>] tun_chr_ioctl+0x2f/0x40
>     [<00000000f30f834a>] do_vfs_ioctl+0x1c7/0x1510
>     [<00000000fba062ea>] ksys_ioctl+0x99/0xb0
>     [<00000000b1c1b8d2>] __x64_sys_ioctl+0x78/0xb0
>     [<00000000984cabb9>] do_syscall_64+0x16f/0x580
>     [<000000000bde033d>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>     [<00000000e6ca2d9f>] 0xffffffffffffffff

How is this a leak?  This is in device_add(), not removing the device.
When the structure really is freed then it can be removed.

Or are you triggering an error in device_add() somehow to trigger this
callback?

thanks,

greg k-h

  reply	other threads:[~2019-11-15  3:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14 12:18 [PATCH] drivers/base: Fix memory leak in error paths jouni.hogander
2019-11-15  3:26 ` Greg Kroah-Hartman [this message]
2019-11-15  7:59   ` Jouni Högander
2019-11-15  8:20     ` Greg Kroah-Hartman
2019-11-15 10:05       ` Jouni Högander
2019-11-15 10:19         ` Greg Kroah-Hartman
2019-11-15 10:32           ` Jouni Högander

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20191115032603.GG793701@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=jouni.hogander@unikie.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.