linux-kernel - Re: [PATCH] drm/mgag200: fix memory leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPM=9tzwUOKzJwvH1CO2u7YDCUoai3XxAdWZzA2_c44e4XfEEA@mail.gmail.com>
Date:	Mon, 14 Sep 2015 20:04:08 +1000
From:	Dave Airlie <airlied@...il.com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Sudip Mukherjee <sudipm.mukherjee@...il.com>,
	David Airlie <airlied@...ux.ie>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	LKML <linux-kernel@...r.kernel.org>,
	dri-devel <dri-devel@...ts.freedesktop.org>,
	Archit Taneja <archit@...com>
Subject: Re: [PATCH] drm/mgag200: fix memory leak

>
>> If drm_fb_helper_alloc_fbi() fails then we were directly returning
>> without freeing sysram. Also if drm_fb_helper_alloc_fbi() succeeds but
>> mgag200_framebuffer_init() fails then we were not releasing sysram and
>> we were not releasing fbi helper also.
>>
>> Signed-off-by: Sudip Mukherjee <sudip@...torindia.org>
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_fb.c | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c
>> index 87de15e..5fe476a 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_fb.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
>> @@ -189,14 +189,16 @@ static int mgag200fb_create(struct drm_fb_helper *helper,
>>               return -ENOMEM;
>>
>>       info = drm_fb_helper_alloc_fbi(helper);
>> -     if (IS_ERR(info))
>> -             return PTR_ERR(info);
>> +     if (IS_ERR(info)) {
>> +             ret = PTR_ERR(info);
>> +             goto err_alloc_fbi;
>> +     }
>>
>>       info->par = mfbdev;
>>
>>       ret = mgag200_framebuffer_init(dev, &mfbdev->mfb, &mode_cmd, gobj);
>>       if (ret)
>> -             return ret;
>> +             goto err_framebuffer_init;
>>
>>       mfbdev->sysram = sysram;
>>       mfbdev->size = size;
>> @@ -226,6 +228,13 @@ static int mgag200fb_create(struct drm_fb_helper *helper,
>>       DRM_DEBUG_KMS("allocated %dx%d\n",
>>                     fb->width, fb->height);
>>       return 0;
>> +
>> +err_framebuffer_init:
>> +     drm_fb_helper_release_fbi(helper);
>> +
>> +err_alloc_fbi:
>> +     vfree(sysram);
>> +     return ret;
>>  }
>>
>>  static int mga_fbdev_destroy(struct drm_device *dev,
>
> There's a new regression: v4.3-rc1 crashes on bootup on non-supported hardware, if
> CONFIG_DRM_MGAG200=y (built into the kernel).

Archit, I'm guessing this is some fallout from the fbdev changes.

There is no reason we should need CONFIG_FB_LITTLE_ENDIAN I don't think.

Dave.

>
> [   10.191561] bus: 'i2c': add device i2c-0
> [   10.227367] mgadrmfb: enable CONFIG_FB_LITTLE_ENDIAN to support this framebuffer
> [   10.235781] [drm:mgag200_modeset_init] *ERROR* mga_fbdev_init failed
> [   10.242992] mgag200 0000:0b:00.0: Fatal error during modeset init: -22
> [   10.250456] kfree_debugcheck: out of range ptr 6b6b6b6b6b6b6b6bh.
> [   10.257378] invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [   10.264730] Modules linked in:
> [   10.268319] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0-rc1-01643-g6013d75-dirty #15
> [   10.277498] Hardware name: Intel Corporation S2600GZ/S2600GZ, BIOS SE5C600.86B.02.02.0002.122320131210 12/23/2013
> [   10.289111] task: ffff88017fb6c040 ti: ffff88017fb70000 task.ti: ffff88017fb70000
> [   10.297611] RIP: 0010:[<ffffffffa493d1a7>]  [<ffffffffa493d1a7>] kfree_debugcheck+0x20/0x25
> [   10.307170] RSP: 0000:ffff88017fb73b28  EFLAGS: 00010086
> [   10.313213] RAX: 0000000000000035 RBX: 6b6b6b6b6b6b6b6b RCX: 0000000000000000
> [   10.321297] RDX: ffffffffa489ac8f RSI: ffffffffa489b27b RDI: ffffffffa489b11e
> [   10.329381] RBP: ffff88017fb73b30 R08: 0000000000000001 R09: 0000000000000000
> [   10.337466] R10: ffffffffa537dec0 R11: 0000000000000000 R12: 0000000000000001
> [   10.345549] R13: ffffffffa4c2c22a R14: 0000000000000202 R15: ffff8807ee3f1018
> [   10.353632] FS:  0000000000000000(0000) GS:ffff88081b200000(0000) knlGS:0000000000000000
> [   10.362812] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   10.369330] CR2: 0000000000000000 CR3: 00000000258c0000 CR4: 00000000001406f0
> [   10.377415] Stack:
> [   10.379761]  6b6b6b6b6b6b6b6b ffff88017fb73b70 ffffffffa493e421 ffff8807ee784ea0
> [   10.388499]  ffff8807ee784e18 0000000000000001 ffff8807ee361060 00000000ffffffea
> [   10.397238]  ffff8807ee3f1018 ffff88017fb73b98 ffffffffa4c2c22a ffff8807ee784e18
> [   10.405968] Call Trace:
> [   10.408804]  [<ffffffffa493e421>] kfree+0x5a/0x195
> [   10.414256]  [<ffffffffa4c2c22a>] drm_fb_helper_crtc_free+0x28/0x75
> [   10.421368]  [<ffffffffa4c2cbbd>] drm_fb_helper_fini+0x6b/0x6e
> [   10.427996]  [<ffffffffa4ce522d>] mgag200_fbdev_fini+0x8a/0xb9
> [   10.434621]  [<ffffffffa4ce0a17>] mgag200_driver_unload+0x23/0x43
> [   10.441539]  [<ffffffffa4ce0ee1>] mgag200_driver_load+0x4aa/0x4bc
> [   10.448458]  [<ffffffffa4c3537c>] drm_dev_register+0x6a/0xab
> [   10.454889]  [<ffffffffa4c36e42>] drm_get_pci_dev+0xe8/0x1ab
> [   10.461322]  [<ffffffffa4ce4b73>] mga_pci_probe+0xa1/0xaa
> [   10.467465]  [<ffffffffa4b785ba>] pci_device_probe+0x7e/0xe8
> ...
>
> Thanks,
>
>         Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/