Re: Win32 : SOCKET type sign causes malfunctions all over the DLL on libssh 0.4.4

[Aris Adamantiadis <aris@xxxxxxxxxxxx>]

Hi Andrea,

Thanks for your report, we opened a ticket about this. It seems other
libraries work around this problem by defining a constant INVALID_SOCKET
and comparing to this constant rather than doing a comparison with 0.

kr,

Aris

Andrea Moretto a écrit :
> Hi all!
> 
>   I uncovered that there is a serious issue that affects the libssh
> when compiling it on win environments, as of latest stable 0.4.4 release.
> 
> If you check the definition of socket_t type in libssh.h :
> 
>   /* Socket type */
>   #ifdef _WIN32
>   #define socket_t SOCKET
>   #else
>   typedef int socket_t;
>   #endif
> 
> It defines the socket_t type as SOCKET, which is unsigned in winsock. While the
> sockets in Unices environments are commonly integer types.
> This means that a lot of comparisons that assume it as signed (as in Unices)
> do actually fail in windows platforms.
> 
> For instance check the bsd_poll function body : 
> 
> static int bsd_poll(ssh_pollfd_t *fds, nfds_t nfds, int timeout) {
>   fd_set readfds, writefds, exceptfds;
>   struct timeval tv, *ptv;
>   socket_t max_fd;
>   int rc;
>   nfds_t i;
> 
>   if (fds == NULL) {
>       errno = EFAULT;
>       return -1;
>   }
> 
>   FD_ZERO (&readfds);
>   FD_ZERO (&writefds);
>   FD_ZERO (&exceptfds);
> 
>   /* compute fd_sets and find largest descriptor */
>   for (max_fd = -1, i = 0; i < nfds; i++) {
>       if (fds[i].fd < 0) {
>           continue;
>       }
> 
>       if (fds[i].events & (POLLIN | POLLRDNORM)) {
>           FD_SET (fds[i].fd, &readfds);
>       }
>       if (fds[i].events & (POLLOUT | POLLWRNORM | POLLWRBAND)) {
>           FD_SET (fds[i].fd, &writefds);
>       }
>       if (fds[i].events & (POLLPRI | POLLRDBAND)) {
>           FD_SET (fds[i].fd, &exceptfds);
>       }
>       if (fds[i].fd >= max_fd &&                                  HERE!!!!!!!!!
>           (fds[i].events & (POLLIN | POLLOUT | POLLPRI |
>                             POLLRDNORM | POLLRDBAND |
>                             POLLWRNORM | POLLWRBAND))) 
>       {
>           max_fd = fds[i].fd;
>       }
>   }
> .
> .
> .
> 
> The comparison fds[i].fd >= max_fd assumes both of them to be signed ints,
> and the invalid socket to be -1. In windows platforms that comparison fails
> because the invalid socket value is 0xFFFFFFFF, so that comparison is never
> true.
> 
> I also read a message in the mailing list reporting a user complaining that
> channel_poll is not working on windows platforms, while it works properly on unices.
> I debugged the DLL and found out it is due to the unsignedness of socket_t.
> 
> I temporarily fixed those issue type-defiinig socket_t as the following:
> 
>   /* Socket type */
>   #ifdef _WIN32
>   #define socket_t long
>   #else
>   typedef int socket_t;
>   #endif
> 
> Of course this is (and must be) only a temporary fix, because SOCKET can get over
> 0x10000000, causing issues.
> 
> Hope this helps!
> 
> Cheers,
> 
>   Andrea
> 
> 
> Andrea Moretto
> moretto.andrea@xxxxxxxxx
> -------------------------------------------------------
> CONFIDENTIALITY NOTICE
> This message and its attachments are addressed solely to the persons
> above and may contain confidential information. If you have received
> the message in error, be informed that any use of the content hereof
> is prohibited. Please return it immediately to the sender and delete
> the message.
> -------------------------------------------------------
> 
>