From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933369AbcKJNuK (ORCPT ); Thu, 10 Nov 2016 08:50:10 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:34471 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932915AbcKJNuI (ORCPT ); Thu, 10 Nov 2016 08:50:08 -0500 Date: Thu, 10 Nov 2016 13:50:00 +0000 From: Sudip Mukherjee To: Arnd Bergmann Cc: Jann Horn , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-parport@lists.infradead.org, Andy Lutomirski Subject: Re: [PATCH v2] ppdev: fix double-free of pp->pdev->name Message-ID: <20161110134959.GA27565@sudip-tp> References: <1477865964-4497-1-git-send-email-jann@thejh.net> <2721049.iK2v6rcyvW@wuerfel> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2721049.iK2v6rcyvW@wuerfel> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Nov 10, 2016 at 02:18:12PM +0100, Arnd Bergmann wrote: > On Sunday, October 30, 2016 11:19:24 PM CET Jann Horn wrote: > > diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c > > index d23368874710..6af1ce04b3da 100644 > > --- a/drivers/char/ppdev.c > > +++ b/drivers/char/ppdev.c > > @@ -748,10 +748,7 @@ static int pp_release(struct inode *inode, struct file *file) > > } > > > > if (pp->pdev) { > > - const char *name = pp->pdev->name; > > - > > parport_unregister_device(pp->pdev); > > - kfree(name); > > pp->pdev = NULL; > > pr_debug(CHRDEV "%x: unregistered pardevice\n", minor); > > } > > > > I took a closer look at this and found that we also leak the name > that is passed in register_device() in the same file: > > name = kasprintf(GFP_KERNEL, CHRDEV "%x", minor); > ... > pdev = parport_register_dev_model(port, name, &ppdev_cb, minor); > > parport_register_dev_model() copies the name using kstrdup() and > we should really free it after parport_register_dev_model(). yes. I missed that while converting the driver to use device model. Thanks. I will send a patch to fix this tonight. > > It's not a huge problem, just leaking a few bytes of memory, but > the extra kfree() probably came from this confusion. No, it was the old code which was not in the device-model. The old code uses parport_register_device() which just uses the same string that is passed to it and so it was freed while releasing the device. Regards Sudip