mirrors/wayland
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/wayland/wayland.git synced 2025-10-29 05:40:16 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

connection: avoid calling memcpy on NULL, 0

Browse source 
Due to what is arguably a mistake in the C language specification,
passing NULL to memcpy and friends is undefined behavior (UB) even when
the count is 0. C additionally mistakenly leaves NULL + 0 and NULL -
NULL undefined. (C++ fixes this mistake.) These are very problematic
because (NULL, 0) is a natural representation of the empty slice.

Some details:
https://github.com/llvm/llvm-project/issues/49459
https://www.imperialviolet.org/2016/06/26/nonnull.html

Unfortunately, despite how clearly this is a mistake, glibc headers and
GCC now try to exploit this specification mistake and will miscompile
code, so C projects need to workaround this. In particular, UBSan from
Clang will flag this as a bug (although Clang itself has the good sense
to never lean on this bug). We've run into a few UBSan errors in
Chromium stemming from Wayland's memcpy calls. Add runtime guards as
needed to avoid these cases.

Note: Chromium's copy of wayland has
https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/188
applied. It is possible the ring_buffer_copy UB cases are only reachable
with that MR applied, I'm not sure. But it seemed simplest to just add
the fix to wayland as-is. Then when/if that MR lands, it will pick this
up.

Signed-off-by: David Benjamin <davidben@google.com>
This commit is contained in:
David Benjamin 2023-11-07 22:38:52 -05:00
parent edb943dc64
commit 50ea9c5b1c
1 changed files with 8 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

9
src/connection.c
View file

@ -83,6 +83,9 @@ ring_buffer_put(struct wl_ring_buffer *b, const void *data, size_t count)
return -1;
}
if (count == 0)
return 0;
head = MASK(b->head);
if (head + count <= sizeof b->data) {
memcpy(b->data + head, data, count);
@ -150,6 +153,9 @@ ring_buffer_copy(struct wl_ring_buffer *b, void *data, size_t count)
{
uint32_t tail, size;
if (count == 0)
return;
tail = MASK(b->tail);
if (tail + count <= sizeof b->data) {
memcpy(data, b->data + tail, count);
@ -1186,7 +1192,8 @@ serialize_closure(struct wl_closure *closure, uint32_t *buffer,
if (p + div_roundup(size, sizeof *p) > end)
goto overflow;
memcpy(p, closure->args[i].a->data, size);
if (size != 0)
memcpy(p, closure->args[i].a->data, size);
p += div_roundup(size, sizeof *p);
break;
default: