linux-kernel - Re: [PATCH] kdesu broken

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: <20090727234100.40157e06@lxorguk.ukuu.org.uk>
Date:	Mon, 27 Jul 2009 23:41:00 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Ray Lee <ray-lk@...rabbit.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] kdesu broken

> No guarantees. It may or may not compile. It may or may not make any 
> difference. It might do unspeakable things to your pets. It really is 
> meant to be an example of what a "flush" function could/should do. There 
> are probably other things/buffers that may need flushing.

Wrong end of the stick. close() already flushes the transmit buffers
properly (and passes the SuS test suite tests on behaviour). Its the
receive the other end you need to sync which isn't an assumption made or
something with any meaning on real hardware.

> +/* This should probably be a generic function */
> +static void flush_delayed_work(struct delayed_work *work)
> +{
> +	if (cancel_delayed_work(work)) {
> +		schedule_delayed_work(work, 0);
> +		flush_scheduled_work();
> +	}

If the ldisc is busy the scheduled work will not flush the data to the
other tty. The tty->low_latency fix actually works better than this
because it makes tty_flip_buffer_push() force the work through that can
be done as it calls the ldisc work synchronously.

> + *	Called for every close(), whether the last or not

The pty case is "special" and seems to be an accident of implementations
in history. The problem on the pty side is on the input not the output
side.

A normal tty write goes from the ldisc to the device, and in that case
the close behaviour is defined by the close delay and FIFO flush in the
driver. In other words it is already done correctly, only done for the
last close (some apps rely on that as a sneaky way to keep the main
thread from blocking).

The pty race is quite different

A write to the pty is guaranteed to correctly have completed on the
close(). The data may however not have been received by the other end as
the ldisc could be busy or not yet scheduled. If you like the data is
sitting on the virtual wire". Or more exactly its in the "not yet fed to
the ldisc" buffers on the other end.

Adding the EOFPENDING/EOF flag fixes the detection of the EOF by the
receiver but doesn't guarantee the sender blocks. If the sender also
waits for the EOF flag to be set then the sender knows the receiving
ldisc has got all the data. Thats a fairly small mod from the existing
long patch.

Except: The other end could also be in EOFPENDING/EOF wait with both ends
blocked by the ldisc and not accepting data. In which case you just
deadlocked. The close() blocks forever waiting for another end which may
also be close() blocked in the other direction isn't a viable
implementation although one I'm quite keen to try out on a Mac or BSD box
out of curiosity.

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