libc: avoid race using Libc::suspend with pthreads

TOCTTOU bug, in our case time of check to time of sleep bug
This commit is contained in:
Alexander Boettcher 2017-02-20 13:30:36 +01:00 committed by Christian Helmuth
parent 553a4222f4
commit 1a6963813c
6 changed files with 98 additions and 24 deletions

View File

@ -23,7 +23,8 @@ int _nanosleep(const struct timespec *req, struct timespec *rem)
{
unsigned long sleep_ms = req->tv_sec*1000 + req->tv_nsec/1000000;
do { sleep_ms = Libc::suspend(sleep_ms); } while (sleep_ms);
struct Check : Libc::Suspend_functor { bool suspend() override { return true; } } check;
do { sleep_ms = Libc::suspend(check, sleep_ms); } while (sleep_ms);
if (rem) {
rem->tv_sec = 0;

View File

@ -269,9 +269,19 @@ _select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
Timeout(timeval *tv) : _tv(tv) { }
} timeout { tv };
do {
timeout.duration = Libc::suspend(timeout.duration);
} while (!timeout.expired() && select_cb->nready == 0);
struct Check : Libc::Suspend_functor {
struct Timeout *timeout;
Libc::Select_cb *select_cb;
Check(Timeout *timeout, Libc::Select_cb * select_cb)
: timeout(timeout), select_cb(select_cb) { }
bool suspend() override {
return !timeout->expired() && select_cb->nready == 0; }
} check ( &timeout, &*select_cb );
while (!timeout.expired() && select_cb->nready == 0)
timeout.duration = Libc::suspend(check, timeout.duration);
select_cb_list.remove(&(*select_cb));

View File

@ -283,8 +283,15 @@ static int read_sockaddr_in(int read_fd, Socket_context *context,
if (!addrlen || *addrlen <= 0) return Errno(EINVAL);
if (read_fd == -1) return Errno(ENOTCONN);
struct Check : Libc::Suspend_functor {
Socket_context *context;
Check (Socket_context *context) : context (context) { }
bool suspend() override {
return !context->remote_read_ready(); }
} check ( context );
while (block_for_read && !context->remote_read_ready())
Libc::suspend();
Libc::suspend(check);
Sockaddr_string addr_string;
int const n = read(read_fd, addr_string.base(), addr_string.capacity() - 1);

View File

@ -280,7 +280,7 @@ struct Libc::Pthreads
p->lock.unlock();
}
unsigned long suspend_myself(unsigned long timeout_ms)
unsigned long suspend_myself(Suspend_functor & check, unsigned long timeout_ms)
{
Pthread myself { timer_accessor, timeout_ms };
{
@ -289,7 +289,10 @@ struct Libc::Pthreads
myself.next = pthreads;
pthreads = &myself;
}
myself.lock.lock();
if (check.suspend())
myself.lock.lock();
{
Genode::Lock::Guard g(mutex);
@ -449,9 +452,13 @@ struct Libc::Kernel
*/
static void _user_entry(Libc::Kernel *kernel)
{
struct Check : Suspend_functor {
bool suspend() override { return true; }
} check;
kernel->_app_code->execute();
kernel->_app_returned = true;
kernel->_suspend_main(0);
kernel->_suspend_main(check, 0);
}
bool _main_context() const { return &_myself == Genode::Thread::myself(); }
@ -484,8 +491,12 @@ struct Libc::Kernel
_longjmp(_user_context, 1);
}
unsigned long _suspend_main(unsigned long timeout_ms)
unsigned long _suspend_main(Suspend_functor &check,
unsigned long timeout_ms)
{
if (!check.suspend())
return 0;
if (timeout_ms > 0)
_main_timeout.timeout(timeout_ms);
@ -562,7 +573,7 @@ struct Libc::Kernel
/**
* Suspend this context (main or pthread)
*/
unsigned long suspend(unsigned long timeout_ms)
unsigned long suspend(Suspend_functor &check, unsigned long timeout_ms)
{
if (timeout_ms > 0
&& timeout_ms > _timer_accessor.timer().max_timeout()) {
@ -573,8 +584,8 @@ struct Libc::Kernel
timeout_ms = min(timeout_ms, _timer_accessor.timer().max_timeout());
}
return _main_context() ? _suspend_main(timeout_ms)
: _pthreads.suspend_myself(timeout_ms);
return _main_context() ? _suspend_main(check, timeout_ms)
: _pthreads.suspend_myself(check, timeout_ms);
}
unsigned long current_time()
@ -669,12 +680,12 @@ static void resumed_callback() { kernel->entrypoint_resumed(); }
void Libc::resume_all() { kernel->resume_all(); }
unsigned long Libc::suspend(unsigned long timeout_ms)
unsigned long Libc::suspend(Suspend_functor &s,
unsigned long timeout_ms)
{
return kernel->suspend(timeout_ms);
return kernel->suspend(s, timeout_ms);
}
unsigned long Libc::current_time()
{
return kernel->current_time();

View File

@ -47,7 +47,8 @@ namespace Libc {
* or as separate pthread. This function returns after the libc kernel
* resumed the user context execution.
*/
unsigned long suspend(unsigned long timeout_ms = 0UL);
struct Suspend_functor { virtual bool suspend() = 0; };
unsigned long suspend(Suspend_functor &, unsigned long timeout_ms = 0UL);
/**
* Get time since startup in ms

View File

@ -290,24 +290,61 @@ ssize_t Libc::Vfs_plugin::write(Libc::File_descriptor *fd, const void *buf,
}
typedef Vfs::File_io_service::Read_result Result;
struct Read_check : Libc::Suspend_functor {
Vfs::Vfs_handle * handle;
void * buf;
::size_t * count;
Vfs::file_size * out_count;
Result * out_result;
Read_check(Vfs::Vfs_handle * handle, void * buf, ::size_t * count,
Vfs::file_size * out_count, Result * out_result)
: handle(handle), buf(buf), count(count), out_count(out_count),
out_result(out_result)
{ }
};
ssize_t Libc::Vfs_plugin::read(Libc::File_descriptor *fd, void *buf,
::size_t count)
{
typedef Vfs::File_io_service::Read_result Result;
Vfs::Vfs_handle *handle = vfs_handle(fd);
Vfs::file_size out_count = 0;
Result out_result = Result::READ_OK;
while (!handle->fs().queue_read(handle, (char *)buf, count,
out_result, out_count))
Libc::suspend();
out_result, out_count)) {
struct Check : Read_check {
Check(Vfs::Vfs_handle * handle, void * buf, ::size_t * count,
Vfs::file_size * out_count, Result * out_result)
: Read_check (handle, buf, count, out_count, out_result) { }
bool suspend() override {
return !handle->fs().queue_read(handle, (char *)buf, *count,
*out_result, *out_count); }
} check ( handle, buf, &count, &out_count, &out_result);
Libc::suspend(check);
}
while (out_result == Result::READ_QUEUED) {
out_result = handle->fs().complete_read(handle, (char *)buf, count, out_count);
if (out_result == Result::READ_QUEUED)
Libc::suspend();
struct Check : Read_check {
Check(Vfs::Vfs_handle * handle, void * buf, ::size_t * count,
Vfs::file_size * out_count, Result * out_result)
: Read_check (handle, buf, count, out_count, out_result) { }
bool suspend() override {
*out_result = handle->fs().complete_read(handle, (char *)buf,
*count, *out_count);
/* suspend me if read is still queued */
return *out_result == Result::READ_QUEUED;
}
} check ( handle, buf, &count, &out_count, &out_result);
Libc::suspend(check);
}
switch (out_result) {
@ -788,8 +825,15 @@ int Libc::Vfs_plugin::select(int nfds,
FD_SET(fd, readfds);
++nready;
} else {
struct Check : Libc::Suspend_functor {
Vfs::Vfs_handle * handle;
Check(Vfs::Vfs_handle * handle) : handle (handle) { }
bool suspend() override {
return !handle->fs().notify_read_ready(handle); }
} check ( handle );
while (!handle->fs().notify_read_ready(handle))
Libc::suspend();
Libc::suspend(check);
}
}