#16 1 portability problem and 2 serious bugs + fixes

open
nobody
None
5
2005-08-05
2005-08-05
SODA Noriyuki
No

while seeking the reason why tsocks-1.8beta5 doesn't
work with
NetBSD's stock ftp client (in passive mode), I found 1
portability
problem about connect() implementation and 2 serious
bugs in poll()
implementation.

1. poll() bug #1
tsocks assumes that flags in struct
connreq::selectevents are
compatible with flags in struct pollfd::events,
because tsocks.c
line 551 and line 675 directly assign these struct
members each other.
but the assumption is wrong as follows:
connreq::selectevents pollfd::events (on Solaris,
Linux, NetBSD,)
READ 0x01 POLLIN 0x1
WRITE 0x02 POLLOUT 0x4
EXCEPT 0x04 POLLPRI 0x2
also, POLLRDNORM should be treated as READ,
POLLWRNORM and POLLWRBAND should be treated as WRITE,
and POLLRDBAND should be treated as EXCEPT, as well.

the patch to tsocks.h in the attached file fixes this
problem.

2. poll() bug #2
if connection is established and the client polled
for writing,
struct pollfd:revents doesn't correctly updated.

the patch to poll() implementation in the attached
file fixes this problem.

3. connect() portability problem
on NetBSD and some other operating systems, if once
connect(2) is
called with non-blocking mode, subsequent connect(2)
call to same
descriptor doesn't return 0, rather it returns
EINPROGRESS
(Operation now in progress) or EISCONN (Socket is
already connected).
The connect() implementation in tsocks already deals
with the
EINPROGRESS case, but doesn't deal with the EISCONN case.

the patch to connect_server() function in the
attached file fixes
this problem.
perhaps it's better to use "#if defined(__NetBSD__)"
in this case,
but I guess this patch isn't so harmful on other OS.

Discussion

  • SODA Noriyuki
    SODA Noriyuki
    2005-08-05

    bug fixes

     
    Attachments
  • SODA Noriyuki
    SODA Noriyuki
    2005-08-05

    Logged In: YES
    user_id=1324640

    I forgot to mention that at least WRITE case is really
    critical in the problem #1,
    since the value is refered in line 660.

     
  • SODA Noriyuki
    SODA Noriyuki
    2005-08-05

    Logged In: YES
    user_id=1324640

    Second thought.
    It's better to use the following change:
    "ufds[i].revents |= (conn->selectevents & WRITE);"
    insead of "ufds[i].revents |= POLLOUT;" in the second part
    of the patch