[PATCH] Disable AF_UNIX handshake with setsockopt(..., SO_PEERCRED, ...)
Corinna Vinschen
corinna-cygwin@cygwin.com
Fri Oct 10 18:04:00 GMT 2014
On Oct 10 18:36, Christian Franke wrote:
> Corinna Vinschen wrote:
> >I was just looking into applying your patch when I got thinking over the
> >change in select.cc once more. You're setting the connect_state from
> >connect_pending to connected there when there's something to read on the
> >socket.
> >
> >This puzzles me. A completed connection attempt should set the
> >write_selected flag (see function peek_socket).
>
> No, peek_socket() does not change write_selected. It sets write_read if
> write_selected is set.
Uh, sorry, I mistyped. You're right, of course.
> > The AF_LOCAL handling
> >in the
> >
> > if (me->write_selected && me->write_ready)
> >
> >case in set_bits should cover this. What situation is your special case
> >covering which is not already covered by the write_selected case?
>
> If only read status is requested via select()/poll(), write_selected is
> always false and the connect_pending=>connected transition is never done.
>
> After a nonblocking connect(), postfix calls poll() with pollfd.events =
> POLLIN only. If poll() succeeds, it calls recv(). This fails with ENOTCONN
> because the state is still connect_pending.
Oh. So it doesn't check if the connect succeeded? Does it check the
poll result for POLLERR or does it explicitely check for revents==POLLIN?
Hmm.
[...time passes...]
It looks like you catched a long-standing bug here.
This isn't even AF_LOCAL specific. The original comment in the
write_selected branch is misleading: The AF_LOCAL specific part is just
the call to af_local_connect, not setting the connect_state. There was
a previous, longer comment at one point which I shortened for no good
reason in 2005:
/* eid credential transaction on successful non-blocking connect.
Since the read bit indicates an error, don't start transaction
if it's set. */
However, If I'm not completely mistaken, your patch would only work in
the aforementioned scenario if setsockopt(SO_PEERCRED) has been called.
Otherwise the handshake would be skipped on the connect side and thus
the handshake would fail on the server side. There's also the problem
that read_ready may indicate an error. And POLLERR is only set if the
socket is polled for POLLOUT so a failing connect would go unnoticed.
In short, the whole code is written under the assumption that any sane
application calling nonblocking connect would always call select/poll to
check if connect succeeded in the first place. Obviously, as postfix
shows, this is a wrong assumption.
I'm not yet sure how to fix this, but I'll look into this next week.
Thanks,
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20141010/924cd2db/attachment.sig>
More information about the Cygwin-patches
mailing list