linux-kernel - Re: [PATCH] usb-serial regression fix

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: <20070313153050.GA14687@blackbean.org>
Date:	Tue, 13 Mar 2007 08:30:50 -0700
From:	Jim Radford <radford@...ckbean.org>
To:	Mark Lord <lkml@....ca>
Cc:	Greg KH <greg@...ah.com>, linux-usb-devel@...ts.sourceforge.net,
	Oliver Neukum <oneukum@...e.de>, Adrian Bunk <bunk@...sta.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb-serial regression fix

On Tue, Mar 13, 2007 at 09:55:41AM -0400, Mark Lord wrote:
> Jim Radford wrote:
> >On Mon, Mar 12, 2007 at 09:55:14PM -0400, Mark Lord wrote:
> >
> >>So where does the memory get freed -- the structure pointed at
> >>by the serial->port[i] thingie ?  It's not a leak, is it?
> >
> >It gets free'd through device_unregister
> >
> >    for (i = 0; i < num_ports; ++i) {
> >       ...
> >       port->dev.release = &port_release;
> >       ...
> >       retval = device_register(&port->dev);
> >
> >which means that until all the drivers get converted to use
> >->port_probe() and ->port_remove() (which gets called by
> >device_unregister) and stop using the ->port[] array in ->shutdown()
> >we need to have ->shutdown() called before device_unregister.
> >
> >>>>>Look at changeset d9a7ecacac5f8274d2afce09aadcf37bdb42b93a in Linus's
> >>>>>tree from Jim Radford:
> >
> >>>>>http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d9a7ecacac5f8274d2afce09aadcf37bdb42b93a

> >So, this patch should be reverted for now.

> Okay, so.. Jim, could you spell it out for us now?

> I'm confused.

> Based on the current 2.6.21-rc3-git*, tell us *exactly* what (if
> any) needs to be reverted, and *exactly* which (if any) of my
> suggested patches to apply ?

The simple change to not NULL the ->port[] array after unregister was
not quite enough, as ->shutdown() would, as hinted at by Mark, then
access kfree'd memory.

The best thing to do is to just revert
d9a7ecacac5f8274d2afce09aadcf37bdb42b93a and work out a better fix for
the next release.  My patches to fix ftdi_sio no longer require this
patch, so it is pointless at this late state if it breaks *anything*,
and it does.


This patch reverts d9a7ecacac5f8274d2afce09aadcf37bdb42b93a since it
breaks drivers that need to access the ->port[] array in shutdown
(most of them).

Signed-Off: Jim Radford <radford@...ckbean.org>

--- b/drivers/usb/serial/usb-serial.c
+++ a/drivers/usb/serial/usb-serial.c
@@ -137,6 +135,11 @@
 
 	dbg("%s - %s", __FUNCTION__, serial->type->description);
 
+	serial->type->shutdown(serial);
+
+	/* return the minor range that this device had */
+	return_serial(serial);
+
 	for (i = 0; i < serial->num_ports; ++i)
 		serial->port[i]->open_count = 0;
 
@@ -147,12 +150,6 @@
 			serial->port[i] = NULL;
 		}
 
-	if (serial->type->shutdown)
-		serial->type->shutdown(serial);
-
-	/* return the minor range that this device had */
-	return_serial(serial);
-
 	/* If this is a "fake" port, we have to clean it up here, as it will
 	 * not get cleaned up in port_release() as it was never registered with
 	 * the driver core */



-
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/