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]

Date:	Tue, 28 Jul 2009 18:06:49 +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

> Imagine being in 'select()' (or read, for that matter), and getting EINTR 
> due to SIGCHLD. What is the correct expectations?

If you are asking that question I don't think you understand the bug
report.

> The correct expectation is that the select() (or read()) should have 
> returned any data that it saw _before_ it returns EINTR.

read() handles that correctly, has always done so.

emacs from the traces does this

	set O_NDELAY
	wait for SIGCLD
	read()
		EAGAIN
		shit_myself();

The EWOULDBLOCK is perfectly correctly reporting that at that precise
instant no data is available.

Correctly written code does this

	loop: 	read()
		EAGAIN
		poll
		goto loop

and in that case our code all works correctly.

If you put the whole thing on a timeline it looks like this (without
lowlatency being enabled but with the EOF thing fixed)


Slave			Emacs		Kernel

write "errors"
					Queue to tty->buf
close
					This is all the data
					Other end closed
exit
			SIGCLD
			read pty
			EWOULDBLOCK
			shit myself
					schedule n_tty ldisc
					queue bytes to parent

			died rather than waited

Had emacs used poll() properly then it ought to have all worked, although
a spot of review of that wouldn't be a bad thing.

Also calling into the n_tty ldisc side processing in the receiver
unfortunately opens up this stuff


	** interrupt path **
	data received
	queue to tty->buf, and wake

	** hangup **
	closes the ldisc down
	waits for the workqueue to complete
	physical instance goes away

	** a read already in progress **
	(new reads will go via tty_hung_up_write())
	process n_tty ldisc
		(which is already closed)
	echo char
		write to non-existant device
		jump to fishkill (or exploitville or wherever)

Because the hangup code, ldisc and open/close code all work on the basis
that

	- a hang up means the caller will not call tty_flip_buffer_push
	  after calling tty_hangup.
	- the only other path (the non low latency path) is the workqueue
	  which it takes care to kill off

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