linux-kernel - Re: [PATCH] n_hdlc fix read and write locking

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: <201010252231.06794.arnd@arndb.de>
Date:	Mon, 25 Oct 2010 22:31:06 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Paul Fulghum <paulkf@...rogate.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] n_hdlc fix read and write locking

On Monday 25 October 2010 20:22:39 Paul Fulghum wrote:
> Fix locking in read and write code of n_hdlc line discipline.
> 
> 2.6.36 replaced lock_kernel() with tty_lock().
> The tty mutex is not dropped automatically when the thread
> sleeps like the BKL. This results in a blocked read or write holding
> the tty mutex and stalling operations by other devices that use
> the tty mutex.
> 
> A review of n_hdlc read and write code shows:
> 1. neither BKL or tty mutex are required for correct operation
> 2. read can block while read data is available if data is posted
>    between availability check and call to interruptible_sleep_on()
> 3. write does not set process state to TASK_INTERRUPTIBLE
>    on each pass through the processing loop which can cause
>    unneeded scheduling of the thread

Right. I must have missed this when I was not checking for
interruptible_sleep_on(). I did systematically check for
this problem with the wait_event family as well as
work_queues, mutexes, semaphores and hand-written schedule
loops, but for some reason I did not check for sleep_on :(

I've double-checked it now, and it seems that all other
instances of sleep_on are waiting for close_wait in
block_til_ready or open functions, and I remember that
I did check those and convinced myself that they are fine.

> Write corrected to set process state to
> TASK_INTERRUPTIBLE on each pass through processing loop.

Would it be possible to express the same using
wait_event_interruptible()?

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