Call for testing: OpenSSH-6.5

[Darren Tucker]

On Thu, Jan 23, 2014 at 04:05:57PM -0800, Hisashi T Fujinaka wrote:
[...]
> No, local SATA disk.

I'm stealing Damien's thunder here since he did most of the work
reproducing and figuring the out the problem, but since he seems to be
missing in action at the moment: we think we figured it out.

Short answer: please try the patch below.

Long answer:
atomicio is a wrapper function around read and write that ensures all of
the data intended to be read or written actually does.  It does that by
passing a function pointer as its first argument, which it then calls for
the action.  If the read or write returns EAGAIN, it uses poll() to wait
for the descriptor to become readable or writeable, which it decides by
looking at the function pointer.  ccp uses atomicio.

What's happening is that in this case the passed function pointer is
not equal to atomicio's idea of of the address of the read function,
thus when we set the poll flags:

	pfd.events = f == read ? POLLIN : POLLOUT;

it means that on the read side, when the buffer fills, it starts waiting
for it to be *writeable*.  This doesn't happen and thus the the copy
stalls.

Why is the pointer different?  It seems that enabling --stack-protector
or similar (which one did configure enable?) turns read() into a macro
that ends up calling a different function which presumably does some
extra checking..  Why doesn't this transform also apply to atomicio.o?
That part is not clear.

There doesn't seem to be any equivalent macro for write(), so inverting
the test seems to fix it for now.  Longer term maybe we don't want to
use function pointers to pass what's essentially a boolean, but that
would be a larger change for another day after the release.

Thanks for your patience with this.  It does seem like a genuine
problem, albeit with a somewhat obscure cause.

Index: atomicio.c
===================================================================
RCS file: /home/dtucker/openssh/cvs/openssh/atomicio.c,v
retrieving revision 1.40
diff -u -p -r1.40 atomicio.c
--- atomicio.c	24 Sep 2010 12:15:11 -0000	1.40
+++ atomicio.c	24 Jan 2014 04:24:50 -0000
@@ -57,7 +57,7 @@ atomicio6(ssize_t (*f) (int, void *, siz
 	struct pollfd pfd;
 
 	pfd.fd = fd;
-	pfd.events = f == read ? POLLIN : POLLOUT;
+	pfd.events = f == vwrite ? POLLOUT : POLLIN;
 	while (n > pos) {
 		res = (f) (fd, s + pos, n - pos);
 		switch (res) {

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.