Archives For

There are a couple of fairly nasty bugs in the LSP sample code from Microsoft in the function WSPSelect, and as far as I can tell, this one hasn’t been reported anywhere before. Since we’re working on an LSP here, I’ve had to fix this bug. 🙂

The bug affected iTunes and safari, which I have noticed some traffic about.

The actual bug is in the three clauses that copy the activations from the provider-socket fd_sets into the client-supplied fd_sets that contain client-visible socket objects

They look like this:
(note that ReadFds is internal, and contains provider sockets, readfds is the user’s parameter, and contains client facing socket descriptors)

And there are three copies of this code for the three fd_sets passed into select.

So what happens if we pass two fd_sets to this code with the same address? Note that in each clause (seperately), the client’s fd_set is zeroed!

So WSASelect(0, &my_sockets, &my_sockets, &my_sockets, NULL) will report the correct count, but only sockets that have exceptions will remain activated when we return to the caller! Moving the FD_ZERO macro expansions and the count captures all above these passes through the fd_sets will fix this bug.

The second one is less severe but still might be harmful; note that the return from the lower provider’s select is a count of activated sockets. This count is not the number of fd_set members that achieved activation, but the number of sockets, so if a socket is repeated for some reason in the incoming fd_set, it will only be counted once on input, but it will be represented twice in our internal array. Microsoft’s sample code will terminate early in this case (note that HandleCount– will be executed twice for one socket), with two activations of one socket, but leaving any remaining events uncopied, My suggestion is to also check whether the we’d be setting the same socket twice into the client’s fd_set, so we can skip double counting as the underlying provider apparently does.

In any case, you should definitely check the code you’re importing. Even OS authors make mistakes, as much as their frequent machismo might suggest otherwise.