Bug #4308
closed
svc_userfds reallocation in svc_add_input() is fragile
100%
Description
Here is how the svc_userfds realloc is implemented in svc_add_input():
446 svc_userfds = (_svc_user_fd_head *)
447 realloc(svc_userfds,
448 svc_nuserfds * sizeof (_svc_user_fd_head));
449
450 if (svc_userfds == NULL) {
451 syslog(LOG_ERR, "svc_add_input: out of memory");
452 errno = ENOMEM;
453 (void) mutex_unlock(&svc_userfds_lock);
454 return ((svc_input_id_t)-1);
455 }
The problem with this code is that after the reallocation failure we will lost the previously allocated svc_userfds. This is both memory leak, and it also could lead to null pointer dereference later (because svc_nuserfds does not reflect what we have now in svc_userfds) - see __is_a_userfd() for an example of the possible null pointer dereference.
Related issues
Updated by Marcel Telka over 9 years ago
- Assignee set to Marcel Telka
The similar issue is in _svc_attribute_new_id() in the same source file - usr/src/lib/libnsl/rpc/svc.c.
Updated by Marcel Telka over 9 years ago
In addition to the issue in Description, there is another possible memory corruption in svc_add_input():
Every entry in svc_userfds contains a linked list of _svc_user_fd_node nodes. If the list is empty, it is initialized using the LIST_CLR() macro. This macro makes the first entry to point to itself. Once the svc_userfds is reallocated in svc_add_input() the pointers will start to point to already freed memory. Here is an example:
> svc_nuserfds::print
0x15
> svc_userfds::print|::array _svc_user_fd_head 0x15|::print -a _svc_user_fd_head
80da150 {
80da150 lnk = {
80da150 next = 0x8070980
80da154 previous = 0x8070980
}
80da158 mask = 0
}
80da15c {
80da15c lnk = {
80da15c next = 0x807098c
80da160 previous = 0x807098c
}
80da164 mask = 0
}
...
80da234 {
80da234 lnk = {
80da234 next = 0x80da234
80da238 previous = 0x80da234
}
80da23c mask = 0
}
80da240 {
80da240 lnk = {
80da240 next = 0x80da240
80da244 previous = 0x80da240
}
80da248 mask = 0
}
>
As we can see, the beginning of the svc_userfds array points to now-random (and likely freed) memory (these entries were initialized before the last realloc() call), while the end of the svc_userfds array points correctly back to the entry addresses (these entries were initialized after the last realloc() call).
Updated by Marcel Telka over 9 years ago
- Status changed from In Progress to Pending RTI
Updated by Electric Monk over 9 years ago
git commit 29d91154fb99ee8e353a5f89cd59be53f2ed8e64
Author: Marcel Telka <marcel.telka@nexenta.com> 4308 svc_userfds reallocation in svc_add_input() is fragile Reviewed by: Ilya Usvyatsky <ilya.usvyatsky@nexenta.com> Reviewed by: Jan Kryl <jan.kryl@nexenta.com> Approved by: Richard Lowe <richlowe@richlowe.net>
Updated by Rich Lowe over 9 years ago
- Status changed from Pending RTI to Resolved
- % Done changed from 0 to 100
- Tags deleted (
needs-triage)