Skip to content

Commit 677454f

Browse files
committed
Revert "libretro-common/queues: bound fifo_write/read + reject SIZE_MAX init"
This reverts commit 1a13965.
1 parent 1a13965 commit 677454f

5 files changed

Lines changed: 3 additions & 374 deletions

File tree

.github/workflows/Linux-libretro-common-samples.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ jobs:
7979
word_wrap_overflow_test
8080
task_queue_title_error_test
8181
tpool_wait_test
82-
fifo_queue_bounds_test
8382
)
8483
8584
# Per-binary run command (overrides ./<binary> if present).

libretro-common/include/queues/fifo_queue.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,6 @@ static INLINE void fifo_clear(fifo_buffer_t *buffer)
126126
/**
127127
* Writes \c size bytes to the given queue.
128128
*
129-
* \c size is silently capped at \c FIFO_WRITE_AVAIL(buffer) --
130-
* the call writes at most that many bytes and discards any
131-
* excess. Callers that need to be sure all bytes are queued
132-
* must gate on \c FIFO_WRITE_AVAIL beforehand. Behaviour is
133-
* undefined if \c buffer is \c NULL.
134-
*
135129
* @param buffer The FIFO queue to write to.
136130
* @param in_buf The buffer to read bytes from.
137131
* @param size The length of \c in_buf, in bytes.
@@ -141,12 +135,6 @@ void fifo_write(fifo_buffer_t *buffer, const void *in_buf, size_t len);
141135
/**
142136
* Reads \c size bytes from the given queue.
143137
*
144-
* \c size is silently capped at \c FIFO_READ_AVAIL(buffer) --
145-
* the call returns at most that many bytes and leaves the
146-
* trailing portion of \c in_buf untouched. Callers that need
147-
* exactly \c size bytes must gate on \c FIFO_READ_AVAIL
148-
* beforehand. Behaviour is undefined if \c buffer is \c NULL.
149-
*
150138
* @param buffer The FIFO queue to read from.
151139
* @param in_buf The buffer to store the read bytes in.
152140
* @param size The length of \c in_buf, in bytes.

libretro-common/queues/fifo_queue.c

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,7 @@
3131

3232
static bool fifo_initialize_internal(fifo_buffer_t *buf, size_t len)
3333
{
34-
uint8_t *buffer;
35-
36-
/* The ring reserves one slot to distinguish empty from full,
37-
* so the actual allocation is (len + 1) bytes. Reject @len
38-
* values that would wrap that addition: SIZE_MAX would
39-
* compute (size_t)0, which calloc(1, 0) is allowed to satisfy
40-
* with a non-NULL pointer to a zero-byte allocation. Letting
41-
* that succeed would leave buf->size == 0 and the next
42-
* fifo_write would divide by zero at the `% buffer->size`
43-
* step. No current caller asks for SIZE_MAX, so the rejection
44-
* is purely defensive. */
45-
if (len >= SIZE_MAX)
46-
return false;
47-
48-
buffer = (uint8_t*)calloc(1, len + 1);
34+
uint8_t *buffer = (uint8_t*)calloc(1, len + 1);
4935

5036
if (!buffer)
5137
return false;
@@ -105,31 +91,8 @@ fifo_buffer_t *fifo_new(size_t len)
10591

10692
void fifo_write(fifo_buffer_t *buffer, const void *in_buf, size_t len)
10793
{
108-
size_t first_write;
94+
size_t first_write = len;
10995
size_t rest_write = 0;
110-
size_t avail;
111-
112-
/* Cap @len at the available space. Existing callers all
113-
* gate on FIFO_WRITE_AVAIL before invoking us, so this is
114-
* a no-op for them; for any caller that doesn't, the
115-
* unbounded branch below would walk off the end of
116-
* @buffer->buffer (the wrap-around copy at line `memcpy(
117-
* buffer->buffer, ..., rest_write)` would write up to
118-
* len - first_write bytes into a buffer of @buffer->size
119-
* total, overrunning by len - size). Worse, the original
120-
* `buffer->end + len > buffer->size` test wraps in size_t
121-
* for huge @len and silently misclassifies the request as
122-
* "fits in one chunk", taking the corrupting first memcpy
123-
* down a path with no wrap-around bound at all. Capping
124-
* here closes both windows. */
125-
avail = FIFO_WRITE_AVAIL(buffer);
126-
if (len > avail)
127-
len = avail;
128-
129-
if (!len)
130-
return;
131-
132-
first_write = len;
13396

13497
if (buffer->end + len > buffer->size)
13598
{
@@ -146,22 +109,8 @@ void fifo_write(fifo_buffer_t *buffer, const void *in_buf, size_t len)
146109

147110
void fifo_read(fifo_buffer_t *buffer, void *in_buf, size_t len)
148111
{
149-
size_t first_read;
112+
size_t first_read = len;
150113
size_t rest_read = 0;
151-
size_t avail;
152-
153-
/* Same rationale as fifo_write: cap @len at what's actually
154-
* available to avoid out-of-buffer copies on a caller that
155-
* forgot to gate on FIFO_READ_AVAIL. Existing callers all
156-
* gate first; this is defensive. */
157-
avail = FIFO_READ_AVAIL(buffer);
158-
if (len > avail)
159-
len = avail;
160-
161-
if (!len)
162-
return;
163-
164-
first_read = len;
165114

166115
if (buffer->first + len > buffer->size)
167116
{

libretro-common/samples/queues/fifo_queue_bounds_test/Makefile

Lines changed: 0 additions & 29 deletions
This file was deleted.

0 commit comments

Comments
 (0)