linux-kernel - Re: [PATCH 4/4] usb: libusual: locking cleanup

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 22 Dec 2007 09:01:50 -0800
From:	Daniel Walker <dwalker@...sta.com>
To:	Pete Zaitcev <zaitcev@...hat.com>
Cc:	akpm@...ux-foundation.org, mingo@...e.hu,
	linux-kernel@...r.kernel.org, linux@...mer.net,
	jonathan@...masters.org, matthias.kaehlcke@...il.com,
	kjwinchester@...il.com
Subject: Re: [PATCH 4/4] usb: libusual: locking cleanup

On Fri, 2007-12-21 at 22:24 -0800, Pete Zaitcev wrote:

> When I tried it, usb-storage would not load with unresolved symbols.
> It happens if child (usu_probe_thread) runs ahead of its parent
> (usb_usual_init -> usb_register -> usu_probe). It's entirely possible,
> depending on your scheduler.
> 
> I hate this down-up trick too, so if you have a better idea, I'm all ears.

This is what you originally had,

static int usu_probe_thread(void *arg)
{

        /* A completion does not work here because it's counted. */
        down(&usu_init_notify);
        up(&usu_init_notify);
...
}

static int __init usb_usual_init(void)
{
        sema_init(&usu_init_notify, 0); <-- Locked init

        rc = usb_register(&usu_driver);
        up(&usu_init_notify);
        return rc;
}

The locked init can easily be an unlocked init combined with a down() ..
So your protecting usb_register() from something else. 

Then in usu_probe_thread() your basically stopping it at the start of
the function with a down(), and the up() is just ancillary .. So you
could easily move the up() further down in the function and still have
the same level of exclusion.. 

static int usu_probe_thread(void *arg)
{

        down(&usu_init_notify);
...
        up(&usu_init_notify);
}

static int __init usb_usual_init(void)
{
        sema_init(&usu_init_notify, 1); <-- Unlocked init

	down(&usu_init_notify);
        rc = usb_register(&usu_driver);
        up(&usu_init_notify);
        return rc;
}

The above protects the same way that your original code did, with the
added benefit of conforming to mutex style usage. The next step is to
convert to the mutex API..

What I've done is all suppose to be mathematical translations, I wasn't
trying to improve the code just make it use a different API..

Daniel

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