ExternalVendorCode/genode
1
0
Fork 0
mirror of https://github.com/genodelabs/genode.git synced 2024-12-21 06:33:31 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

trace_buffer: only iterate after initialization

Browse Source 
There is a race between the trace subject doing the buffer
initialization and the monitor trying to iterate the buffer entries. If
the monitor tries to iterate entries of an uninitialized buffer, it will
read the very first entry twice. The monitor should therefore only start
iteration when the buffer has been initialised.

genodelabs/genode#4513
This commit is contained in:
Johannes Schlatow 2022-05-20 12:00:09 +02:00 committed by Christian Helmuth
parent f87209f822
commit c38b71146b
2 changed files with 28 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
repos/base/include/base/trace/buffer.h
View File

@ -251,6 +251,9 @@ class Genode::Trace::Simple_buffer
public:
/* return invalid entry (checked by last()) */
static Entry invalid() { return Entry(0); }
size_t length() const { return _entry->len; }
char const *data() const { return _entry->data; }
@ -264,8 +267,18 @@ class Genode::Trace::Simple_buffer
bool head() const { return !last() && _entry->head(); }
};
/* Return whether buffer has been initialized. */
bool initialized() const { return _size && _head_offset <= _size; }
/* Return the very first entry at the start of the buffer. */
Entry first() const { return Entry(_entries); }
Entry first() const
{
/* return invalid entry if buffer is uninitialised */
if (!initialized())
return Entry::invalid();
return Entry(_entries);
}
/**
* Return the entry that follows the given entry.
@ -278,14 +291,14 @@ class Genode::Trace::Simple_buffer
Entry next(Entry entry) const
{
if (entry.last() || entry._padding())
return Entry(0);
return Entry::invalid();
if (entry.head())
return entry;
addr_t const offset = (addr_t)entry.data() - (addr_t)_entries;
if (offset + entry.length() + sizeof(_Entry) > _size)
return Entry(0);
return Entry::invalid();
return Entry((_Entry const *)((addr_t)entry.data() + entry.length()));
}
@ -382,11 +395,11 @@ class Genode::Trace::Partitioned_buffer
** Functions called from the TRACE client **
********************************************/
unsigned wrapped() { return _wrapped; }
unsigned wrapped() const { return _wrapped; }
unsigned long long lost_entries() const { return _lost_entries; }
unsigned long long lost_entries() { return _lost_entries; }
Entry first() const { return _consumer().first(); }
Entry first() const { return _consumer().first(); }
bool initialized() const { return _secondary_offset > 0 && _consumer().initialized(); }
/**
* Return the entry that follows the given entry.

13
repos/os/include/trace/trace_buffer.h
View File

@ -24,9 +24,10 @@
class Trace_buffer
{
private:
using Entry = Genode::Trace::Buffer::Entry;
Genode::Trace::Buffer &_buffer;
Genode::Trace::Buffer::Entry _curr { _buffer.first() };
Entry _curr { Entry::invalid() };
unsigned long long _lost_count { 0 };
public:
@ -41,6 +42,9 @@ class Trace_buffer
{
using namespace Genode;
if (!_buffer.initialized())
return;
bool lost = _buffer.lost_entries() != _lost_count;
if (lost) {
warning("lost ", _buffer.lost_entries() - _lost_count,
@ -48,18 +52,17 @@ class Trace_buffer
_lost_count = (unsigned)_buffer.lost_entries();
}
Trace::Buffer::Entry entry { _curr };
Entry entry { _curr };
/**
* Iterate over all entries that were not processed yet.
*
* A note on terminology: The head of the buffer marks the write
* position. The first entry is the one that starts at the lowest
* memory address. The next entry returns an invalid entry called
* if the 'last' end of the buffer (highest address) was reached.
* memory address. The next entry returns an invalid entry
* if the 'last' entry of the buffer (highest address) was reached.
*/
for (; !entry.head(); entry = _buffer.next(entry)) {
/* continue at first entry if we hit the end of the buffer */
if (entry.last())
entry = _buffer.first();