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] [day] [month] [year] [list]
Message-ID: <20150711000343.GA30309@kroah.com>
Date:	Fri, 10 Jul 2015 17:03:43 -0700
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	Johannes Thumshirn <jthumshirn@...e.de>
Cc:	"Hans J. Koch" <hjk@...sjkoch.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] uio: Destroy uio_idr on module exit

On Thu, Jul 09, 2015 at 08:48:36AM +0200, Johannes Thumshirn wrote:
> Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes:
> 
> > On Wed, Jul 08, 2015 at 05:24:46PM +0200, Johannes Thumshirn wrote:
> >> Destroy uio_idr on module exit, reclaiming the allocated memory.
> >> 
> >> This was detected by the following semantic patch (written by Luis Rodriguez
> >> <mcgrof@...e.com>)
> >> <SmPL>
> >> @ defines_module_init @
> >> declarer name module_init, module_exit;
> >> declarer name DEFINE_IDR;
> >> identifier init;
> >> @@
> >> 
> >> module_init(init);
> >> 
> >> @ defines_module_exit @
> >> identifier exit;
> >> @@
> >> 
> >> module_exit(exit);
> >> 
> >> @ declares_idr depends on defines_module_init && defines_module_exit @
> >> identifier idr;
> >> @@
> >> 
> >> DEFINE_IDR(idr);
> >> 
> >> @ on_exit_calls_destroy depends on declares_idr && defines_module_exit @
> >> identifier declares_idr.idr, defines_module_exit.exit;
> >> @@
> >> 
> >> exit(void)
> >> {
> >>  ...
> >>  idr_destroy(&idr);
> >>  ...
> >> }
> >> 
> >> @ missing_module_idr_destroy depends on declares_idr && defines_module_exit && !on_exit_calls_destroy @
> >> identifier declares_idr.idr, defines_module_exit.exit;
> >> @@
> >> 
> >> exit(void)
> >> {
> >>  ...
> >>  +idr_destroy(&idr);
> >> }
> >> 
> >> </SmPL>
> >> 
> >> Signed-off-by: Johannes Thumshirn <jthumshirn@...e.de>
> >
> > Nice work.  Shouldn't we do the same thing for ida_destroy() as well?  I
> > see 4 USB drivers that need this same fix for that structure.
> >
> 
> Can you tell me which? I've send overall 13 patches for this (no series
> as get_maintainers.pl for the series spit out > 30 Recipients so I
> refused to send spam).
> 
> Maybe/probably I forgot one (or two).

You need to look for "ida_destroy" not just "idr_destroy", here's the
ones I found in just a few seconds of looking:

~/linux/gregkh $ cd drivers/usb/
~/linux/gregkh/drivers/usb $ cg ida_destroy
~/linux/gregkh/drivers/usb $ cg DEFINE_IDA
0 chipidea/core.c             597 static DEFINE_IDA(ci_ida);
1 gadget/function/f_hid.c      31 static DEFINE_IDA(hidg_ida);
2 gadget/function/f_printer.c  60 static DEFINE_IDA(printer_ida);
3 gadget/function/rndis.c      67 static DEFINE_IDA(rndis_ida);

There's 4 users that never free memory when the module is unloaded for
the ida structure.

thanks,

greg k-h
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ