1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
|
From a8c7553ec9af6462474524fd2bb4e9a7dc7217dd Mon Sep 17 00:00:00 2001
From: Thomas Lukaszewicz <tluk@chromium.org>
Date: Fri, 21 Jul 2023 23:28:12 +0000
Subject: [wayland] Fix buffer resize crash
Currently chromium wayland clients can crash if a connection buffer
(which is initialized at a capacity of 2^12) is asked to resize
exactly at its resize threshold. This threshold is in steps of
powers of 2.
Take net_size to be the size the connection buffer is required to
support for the current outbound request queue.
This is crash due to certain parts of the code assuming that
net_size can exactly match the capacity of the connection buffer,
while other parts of the code assume the capacity of the buffer
must exceed the net_size. The latter assumption is the root cause
of the crash.
Specifically:
1. ring_buffer_ensure_space() attempts to find the new buffer
`size_bits` required to contain the new `net_size` of the
request buffer [1]
2. This eventually calls into get_max_size_bits_for_size()
which finds the `size_bits` required for `net_size`[2]
- This code assumes that `net_size <= 2^size_bits` in order
to fit the data
3. Back in ring_buffer_ensure_space() the `size_bits` returned
is checked to ensure it can contain `net_size` and if a
resize is needed.
- This code assumes that `net_size < 2^size_bits` in order
to fit the data [3]
- If a resize is not needed, the buffer is checked
whether it can hold `net_size` without resizing and a
crash occurs [4]
The assumptions about buffer capacity differ in 2 and 3, and the
assertion in 3 causes the crash in the associated bug. This only
occurs when `net_size == 2^size_bits` which satisfies the condition
in 2 but fails the condition in 3.
In Lacros code there were a set of users encountering this crash
when buffers were being asked to resize when clients made the
zwp_text_input_v1_set_surrounding_text() with large blocks of
text (typically from a ssh session).
This CL enforces the requirement that the connection buffer must
be greater than the net_size.
For more context see below.
https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/188#note_2010874
[1] https://chromium.googlesource.com/external/anongit.freedesktop.org/git/wayland/wayland/+/refs/heads/chromium_1_21_0/src/connection.c#242
[2] https://chromium.googlesource.com/external/anongit.freedesktop.org/git/wayland/wayland/+/refs/heads/chromium_1_21_0/src/connection.c#183
[3] https://chromium.googlesource.com/external/anongit.freedesktop.org/git/wayland/wayland/+/refs/heads/chromium_1_21_0/src/connection.c#249
[4] https://chromium.googlesource.com/external/anongit.freedesktop.org/git/wayland/wayland/+/refs/heads/chromium_1_21_0/src/connection.c#229
Bug: 1451333
Change-Id: I87853e3ff26c1d693f199fd0045fef796c3730c9
---
src/connection.c | 2 +-
tests/connection-test.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/src/connection.c b/src/connection.c
index 24544d0..276d147 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -180,7 +180,7 @@ get_max_size_bits_for_size(size_t buffer_size)
return 0;
do {
- if (buffer_size <= (((size_t)1) << max_size_bits))
+ if (buffer_size < (((size_t)1) << max_size_bits))
break;
} while (max_size_bits++ < (8 * sizeof(size_t) - 1));
diff --git a/tests/connection-test.c b/tests/connection-test.c
index f5d943c..60c3441 100644
--- a/tests/connection-test.c
+++ b/tests/connection-test.c
@@ -697,6 +697,32 @@ TEST(connection_marshal_big_enough)
free(big_string);
}
+/* Ensures that buffers are resiable to accomodate requests for sizes at the
+ * resize threshold (power of 2). Regression test for crbug.com/1451333. */
+TEST(connection_marshal_resize_at_buffer_growth_threshold)
+{
+ /* A string of lenth 8178 requires a buffer size of exactly 2^13.
+ * TODO(tluk): Determine a way to require a resize at 2^13 in a more robust
+ * way */
+ struct marshal_data data;
+ const int kStrSize = 8178;
+ char *big_string = malloc(kStrSize);
+ assert(big_string);
+
+ memset(big_string, ' ', kStrSize-1);
+ big_string[kStrSize-1] = '\0';
+
+ setup_marshal_data(&data);
+
+ /* Set the max size to 0 (unbounded). */
+ wl_connection_set_max_buffer_size(data.write_connection, 0);
+
+ marshal_send(&data, "s", big_string);
+
+ release_marshal_data(&data);
+ free(big_string);
+}
+
static void
marshal_helper(const char *format, void *handler, ...)
{
--
2.43.0
|