Re: Escaping from blocked send() reprised.

On 2015-01-11 16:36:07 -0500, Noah Misch wrote:
> On Sat, Jan 10, 2015 at 03:25:42AM +0100, Andres Freund wrote:
> > 0001-Allow-latches-to-wait-for-socket-writability-without.patch
> >      Imo pretty close to commit and can be committed independently.
> 
> The key open question is whether all platforms of interest can reliably detect
> end-of-file when poll()ing or select()ing for write only.  Older GNU/Linux
> select() cannot; see attached test program.

Yuck. By my reading that's a violation of posix.

I did test it a bit, and I didn't see problems, but that obviously
doesn't say much about old versions.

Afaics we interestingly don't have any poll-less buildfarm animals that
use unix_latch.c...

> We use poll() there anyway, so the bug in that configuration does not
> affect PostgreSQL.  Is it a bellwether of similar bugs in other
> implementations, bugs that will affect PostgreSQL?

Hm. I can think of two stopgap measures we could add:

1) If we're using select() and WL_SOCKET_WRITEABLE is set without  _READABLE, add a timeout of Min(1s,
Max(passed_timeout,1s)). As the  time spent waiting only for writable normally shouldn't be very long,  that shouldn't
benoticeably bad for power usage.
 
2) Add a SIGPIPE handler that just does a SetLatch(MyLach).

> > This previously had explicitly been forbidden in e42a21b9e6c9, as
> > there was no use case at that point. We now are looking into making
> > FE/BE communication use latches, so it
> 
> Truncated sentence.

Fixed in what I've since pushed (as Heikki basically was ok with the
patch sent a couple months back, modulo some fixes)...

> > +            if (pfds[0].revents & (POLLHUP | POLLERR | POLLNVAL))
> > +            {
> > +                /* EOF/error condition */
> > +                if (wakeEvents & WL_SOCKET_READABLE)
> > +                    result |= WL_SOCKET_READABLE;
> > +                if (wakeEvents & WL_SOCKET_WRITEABLE)
> > +                    result |= WL_SOCKET_WRITEABLE;
> > +            }
> 
> With some poll() implementations (e.g. OS X), this can wrongly report
> WL_SOCKET_WRITEABLE if the peer used shutdown(SHUT_WR).  I tentatively think
> that's acceptable.  libpq does not use shutdown(), and other client interfaces
> would do so at their own risk.  Should we worry about hostile clients creating
> a denial-of-service by causing a server send() to block unexpectedly?
> Probably not; a user able to send arbitrary TCP traffic to the postmaster port
> can already achieve that.

Yea, this doesn't seem particularly concerning.

a) They can just stop consuming writes and use noticeable amounts of
memory by doing output intensive queries. That uses significant os
resources and is much harder to detect - today.

b) does accepting WL_SOCKET_WRITEABLE without _READABLE change anything
here? We already allow _WRITABLE... What happens if you write/send() in
that state, btw?

Greetings,

Andres Freund