[PATCH] KExtendedSocket crashes

[Thiago Macieira]

Adriaan de Groot wrote:
>A FBSD user reported that KSCD with Remote FreeDB enabled crashes if you
> have no network configured on the system. I hunted this down to a NULL
> pointer dereference in KExtendedSocket::connect().
>
>
>If name resolution fails and sets d->resolution to NULL, lookup()
>returns as if it did succeed. SO in connect() when "things are a little
>tricky", using d->resolution->data (kextsock.cpp:1211) crashes.
>Fix there or in connect()? Or in listen()?

The bug seems to be actually somewhere deeper. lookup() isn't supposed to 
return 0 (success) and not set d->resolution. I can't find the error just by 
looking at the code...

>
>THis diff is a fix in lookup() that fakes some stuff:
>
>Index: kextsock.cpp
>===================================================================
>RCS file: /home/kde/kdelibs/kdecore/kextsock.cpp,v
>retrieving revision 1.36.2.2
>diff -r1.36.2.2 kextsock.cpp
>914a915,920
>
>>       if (d->resolution == NULL)
>>         {
>>         setError(IO_LookupError,EAGAIN);
>>         return -1;
>>       }

Please, use unidiffs. IO_LookupError errors don't keep errno value as their 
second value. Instead, I'd set this to EAI_AGAIN.

>I don't know if this affects passive sockets. Perhaps it needs a fix in
>connect() in which case the patch is
>
>@@ -1174,7 +1180,8 @@
>   if (d->status < lookupDone)
>     if (lookup() < 0)
>       return -2;
>-
>+  if (!d->resolution) return -2;
>+
>   addrinfo *p, *q;
>   timeval end, now;
>   // Ok, things are a little tricky here
>
>
>OK to commit to BRANCH? Useful to commit to HEAD? Bandwidth is a little
> short here, I can't suck down HEAD to check how things are there.

In HEAD, it's the same thing. I'll commit to HEAD, if you can't do that. The 
same checking could also be added to KExtendedSocket::listen(int)

Then again, as I said before, the error is somewhere deeper down. 
KExtendedSocket::lookup() will return the return value from 
KExtendedSocket::doLookup(), which in turn returns the error value from 
kde_getaddrinfo (netsupp.cpp)

>In other news:
>
>KExtendedSocket::lookup() is documented to return 0 on success,
>non 0 on failure, but the code elsewhere in KExtendedSocket checks
>for error returns < 0.

At least in HEAD, all the checks are for != 0. As I wrote in the 
documentation, the return value is non zero for errors, but it can also be 
positive. I had intended to write code to translate the error values from 
getaddrinfo() to our own codes.

-- 
  Thiago Macieira - UFOT Registry number: 1001
 thiagom at mail.com
   ICQ UIN: 1967141  PGP/GPG: 0x6EF45358
     Registered Linux user #65028
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: signature
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20021116/ec6ed909/attachment.sig>