linux-kernel - Re: linux-next: manual merge of the ttydev tree with the usb.current tree

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: <200907281410.41449.oliver@neukum.org>
Date:	Tue, 28 Jul 2009 14:10:40 +0200
From:	Oliver Neukum <oliver@...kum.org>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Stephen Rothwell <sfr@...b.auug.org.au>,
	linux-next@...r.kernel.org, linux-kernel@...r.kernel.org,
	Greg KH <greg@...ah.com>
Subject: Re: linux-next: manual merge of the ttydev tree with the usb.current tree

Am Dienstag, 28. Juli 2009 12:26:08 schrieb Alan Cox:
> On Tue, 28 Jul 2009 14:01:10 +1000
>
> Stephen Rothwell <sfr@...b.auug.org.au> wrote:
> > Hi Alan,
> >
> > Today's linux-next merge of the ttydev tree got a conflict in
> > drivers/usb/serial/usb-serial.c between commit
> > a00b8d98aca97bfb6fa983f41dae25b424058592 ("USB: fix usage count in usb
> > serial generic open regarding autoresume") from the usb.current tree	
>
> That looks wrong. The extra fiddling with port.count looks both unsafe
> and incorrect. I've never seen the "autoresume" patch referred to so I've
> no idea what it is trying to do

The problem is subdrivers looking at port->count to determine
whether URBs have to be resubmitted. That must happen once.
Therefore if serial_open() needs to do it itself it must make sure
resume() runs without port->count already incremented.

As serial_open() takes the lock, resume() must not do so.
If however, remote wakeup is used a lock must be taken
to look at port->count reliably. As this is impossible, the patch
implementing use of remote wakeup for the option driver
implements a private flag for opened ports.

This code from option_resume():

for (i = 0; i < serial->num_ports; i++) {
		/* walk all ports */
		port = serial->port[i];
		portdata = usb_get_serial_port_data(port);
		mutex_lock(&port->mutex);

		/* skip closed ports */
		if (!port->port.count) {
			mutex_unlock(&port->mutex);
			continue;
		}

		for (j = 0; j < N_IN_URB; j++) {
			urb = portdata->in_urbs[j];
			err = usb_submit_urb(urb, GFP_NOIO);
			if (err < 0) {
				mutex_unlock(&port->mutex);
				err("%s: Error %d for bulk URB %d",
					 __func__, err, i);
				return err;
			}
		}
		mutex_unlock(&port->mutex);

must not take the port->mutex.

Unfortunately I have no access to my trees today and yesterday.

	Regards
		Oliver

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