Bug #4545
closed
_t_create(): Use after free in error code paths
100%
Description
There is this code in the _t_create() function:
961 ntiptr = add_tilink(fd);
962 if (ntiptr == NULL) {
963 t_errno = TSYSERR;
964 errno = ENOMEM;
965 return (NULL);
966 }
967 sig_mutex_lock(&ntiptr->ti_lock);
968
969 /*
970 * Allocate buffers for the new descriptor
971 */
972 if (_t_alloc_bufs(fd, ntiptr, tiap) < 0) {
973 sv_errno = errno;
974 (void) _t_delete_tilink(fd);
975 t_errno = TSYSERR;
976 sig_mutex_unlock(&ntiptr->ti_lock);
977 errno = sv_errno;
978 return (NULL);
979 }
The ntiptr is allocated at line 961 in add_tilink() and freed at line 974 via _t_delete_tilink(). However, at line 976 the freed memory is referenced.
Similar error handling code is in the _t_create() used several times.
Files
Related issues
Updated by Marcel Telka over 9 years ago
The newly allocated ntiptr (via the add_tilink() function) is added into the hash_bucket array in the add_tilink(). All operations with the hash_bucket array are protected by _ti_userlock mutex. This also includes add_tilink() and find_tilink() functions. The find_tilink() function is used to get a tiptr from the hash_bucket array for the particular fd. Since we hold the _ti_userlock during the _t_create() execution, there is no way how any other thread could obtain a pointer to our newly allocated ntiptr. If such attempt is made, the thread will be blocked on the _ti_userlock mutex lock; even before the find_tilink() is called.
Based on the above, our newly allocated ntiptr is sufficiently protected by the main _ti_userlock mutex, and we do no need to hold ntiptr->ti_lock when we are working with ntiptr in _t_create().
Updated by Marcel Telka over 9 years ago
In addition, none of the code executed after the add_tilink() call in _t_create() requires to hold the ntiptr->ti_lock. This means that we could safely remove the locking/unlocking of the ntiptr->ti_lock from _t_create(). The removal will also fix the problem with use after free.
Updated by Marcel Telka almost 9 years ago
To reproduce the bug use the attached test.c and test.d files and do the following:
# gcc -lnsl -lumem test.c -o test
# export UMEM_DEBUG=default
# dtrace -Cs test.d -c ./test
dtrace: script 'test.d' matched 6 probes
dtrace: allowing destructive actions
t_open failed
CPU ID FUNCTION:NAME
0 64690 _t_delete_tilink:entry
0 64693 sig_mutex_unlock:entry mutex_t {
struct flags = {
uint16_t flag1 = 0xbeef
uint8_t flag2 = 0xad
uint8_t ceiling = 0xde
union mbcp_type_un = {
uint16_t bcptype = 0xbeef
struct mtype_rcount = {
uint8_t count_type1 = 0xef
uint8_t count_type2 = 0xbe
}
}
uint16_t magic = 0xdead
}
union lock = {
struct lock64 = {
uint8_t [8] pad = [ 0xef, 0xbe, 0xad, 0xde, 0xef, 0xbe, 0xad, 0xde ]
}
struct lock32 = {
uint32_t ownerpid = 0xdeadbeef
uint32_t lockword = 0xdeadbeef
}
upad64_t owner64 = 0xdeadbeefdeadbeef
}
upad64_t data = 0xdeadbeefdeadbeef
}
dtrace: pid 2252 has exited
#
We see the mutex passed to sig_mutex_unlock() is already freed (0xdeadbeef).
Updated by Marcel Telka almost 9 years ago
- Status changed from In Progress to Pending RTI
Updated by Electric Monk almost 9 years ago
- Status changed from Pending RTI to Closed
- % Done changed from 0 to 100
git commit d0fcb88af333aa48dd2b958f3681f1b597b924cc
commit d0fcb88af333aa48dd2b958f3681f1b597b924cc
Author: Marcel Telka <marcel.telka@nexenta.com>
Date: 2015-02-11T16:30:40.000Z
4545 _t_create(): Use after free in error code paths
Reviewed by: Garrett D'Amore <garrett@damore.org>
Reviewed by: Dan McDonald <danmcd@omniti.com>
Approved by: Richard Lowe <richlowe@richlowe.net>