Commit | Line | Data |
---|---|---|
84253c8b KC |
1 | .. SPDX-License-Identifier: GPL-2.0 |
2 | ||
98348577 FV |
3 | .. _deprecated: |
4 | ||
84253c8b KC |
5 | ===================================================================== |
6 | Deprecated Interfaces, Language Features, Attributes, and Conventions | |
7 | ===================================================================== | |
8 | ||
9 | In a perfect world, it would be possible to convert all instances of | |
10 | some deprecated API into the new API and entirely remove the old API in | |
11 | a single development cycle. However, due to the size of the kernel, the | |
12 | maintainership hierarchy, and timing, it's not always feasible to do these | |
13 | kinds of conversions at once. This means that new instances may sneak into | |
14 | the kernel while old ones are being removed, only making the amount of | |
15 | work to remove the API grow. In order to educate developers about what | |
16 | has been deprecated and why, this list has been created as a place to | |
17 | point when uses of deprecated things are proposed for inclusion in the | |
18 | kernel. | |
19 | ||
20 | __deprecated | |
21 | ------------ | |
22 | While this attribute does visually mark an interface as deprecated, | |
23 | it `does not produce warnings during builds any more | |
24 | <https://git.kernel.org/linus/771c035372a036f83353eef46dbb829780330234>`_ | |
25 | because one of the standing goals of the kernel is to build without | |
26 | warnings and no one was actually doing anything to remove these deprecated | |
27 | interfaces. While using `__deprecated` is nice to note an old API in | |
28 | a header file, it isn't the full solution. Such interfaces must either | |
29 | be fully removed from the kernel, or added to this file to discourage | |
30 | others from using them in the future. | |
31 | ||
7af51678 KC |
32 | BUG() and BUG_ON() |
33 | ------------------ | |
34 | Use WARN() and WARN_ON() instead, and handle the "impossible" | |
35 | error condition as gracefully as possible. While the BUG()-family | |
36 | of APIs were originally designed to act as an "impossible situation" | |
37 | assert and to kill a kernel thread "safely", they turn out to just be | |
38 | too risky. (e.g. "In what order do locks need to be released? Have | |
39 | various states been restored?") Very commonly, using BUG() will | |
40 | destabilize a system or entirely break it, which makes it impossible | |
41 | to debug or even get viable crash reports. Linus has `very strong | |
42 | <https://lore.kernel.org/lkml/CA+55aFy6jNLsywVYdGp83AMrXBo_P-pkjkphPGrO=82SPKCpLQ@mail.gmail.com/>`_ | |
43 | feelings `about this | |
44 | <https://lore.kernel.org/lkml/CAHk-=whDHsbK3HTOpTF=ue_o04onRwTEaK_ZoJp_fjbqq4+=Jw@mail.gmail.com/>`_. | |
45 | ||
46 | Note that the WARN()-family should only be used for "expected to | |
47 | be unreachable" situations. If you want to warn about "reachable | |
48 | but undesirable" situations, please use the pr_warn()-family of | |
49 | functions. System owners may have set the *panic_on_warn* sysctl, | |
50 | to make sure their systems do not continue running in the face of | |
51 | "unreachable" conditions. (For example, see commits like `this one | |
52 | <https://git.kernel.org/linus/d4689846881d160a4d12a514e991a740bcb5d65a>`_.) | |
53 | ||
84253c8b KC |
54 | open-coded arithmetic in allocator arguments |
55 | -------------------------------------------- | |
56 | Dynamic size calculations (especially multiplication) should not be | |
57 | performed in memory allocator (or similar) function arguments due to the | |
58 | risk of them overflowing. This could lead to values wrapping around and a | |
59 | smaller allocation being made than the caller was expecting. Using those | |
60 | allocations could lead to linear overflows of heap memory and other | |
61 | misbehaviors. (One exception to this is literal values where the compiler | |
3577cdb2 LB |
62 | can warn if they might overflow. However, the preferred way in these |
63 | cases is to refactor the code as suggested below to avoid the open-coded | |
64 | arithmetic.) | |
84253c8b KC |
65 | |
66 | For example, do not use ``count * size`` as an argument, as in:: | |
67 | ||
68 | foo = kmalloc(count * size, GFP_KERNEL); | |
69 | ||
70 | Instead, the 2-factor form of the allocator should be used:: | |
71 | ||
72 | foo = kmalloc_array(count, size, GFP_KERNEL); | |
73 | ||
74 | If no 2-factor form is available, the saturate-on-overflow helpers should | |
75 | be used:: | |
76 | ||
77 | bar = vmalloc(array_size(count, size)); | |
78 | ||
79 | Another common case to avoid is calculating the size of a structure with | |
80 | a trailing array of others structures, as in:: | |
81 | ||
82 | header = kzalloc(sizeof(*header) + count * sizeof(*header->item), | |
83 | GFP_KERNEL); | |
84 | ||
85 | Instead, use the helper:: | |
86 | ||
87 | header = kzalloc(struct_size(header, item, count), GFP_KERNEL); | |
88 | ||
68e4cd17 GS |
89 | .. note:: If you are using struct_size() on a structure containing a zero-length |
90 | or a one-element array as a trailing array member, please refactor such | |
91 | array usage and switch to a `flexible array member | |
92 | <#zero-length-and-one-element-arrays>`_ instead. | |
93 | ||
7929b983 JC |
94 | See array_size(), array3_size(), and struct_size(), |
95 | for more details as well as the related check_add_overflow() and | |
96 | check_mul_overflow() family of functions. | |
84253c8b KC |
97 | |
98 | simple_strtol(), simple_strtoll(), simple_strtoul(), simple_strtoull() | |
99 | ---------------------------------------------------------------------- | |
7929b983 JC |
100 | The simple_strtol(), simple_strtoll(), |
101 | simple_strtoul(), and simple_strtoull() functions | |
84253c8b | 102 | explicitly ignore overflows, which may lead to unexpected results |
7929b983 JC |
103 | in callers. The respective kstrtol(), kstrtoll(), |
104 | kstrtoul(), and kstrtoull() functions tend to be the | |
84253c8b KC |
105 | correct replacements, though note that those require the string to be |
106 | NUL or newline terminated. | |
107 | ||
108 | strcpy() | |
109 | -------- | |
27def953 KC |
110 | strcpy() performs no bounds checking on the destination buffer. This |
111 | could result in linear overflows beyond the end of the buffer, leading to | |
112 | all kinds of misbehaviors. While `CONFIG_FORTIFY_SOURCE=y` and various | |
113 | compiler flags help reduce the risk of using this function, there is | |
114 | no good reason to add new uses of this function. The safe replacement | |
115 | is strscpy(), though care must be given to any cases where the return | |
116 | value of strcpy() was used, since strscpy() does not return a pointer to | |
117 | the destination, but rather a count of non-NUL bytes copied (or negative | |
118 | errno when it truncates). | |
84253c8b KC |
119 | |
120 | strncpy() on NUL-terminated strings | |
121 | ----------------------------------- | |
27def953 KC |
122 | Use of strncpy() does not guarantee that the destination buffer will |
123 | be NUL terminated. This can lead to various linear read overflows and | |
124 | other misbehavior due to the missing termination. It also NUL-pads | |
125 | the destination buffer if the source contents are shorter than the | |
126 | destination buffer size, which may be a needless performance penalty | |
127 | for callers using only NUL-terminated strings. The safe replacement is | |
128 | strscpy(), though care must be given to any cases where the return value | |
129 | of strncpy() was used, since strscpy() does not return a pointer to the | |
130 | destination, but rather a count of non-NUL bytes copied (or negative | |
131 | errno when it truncates). Any cases still needing NUL-padding should | |
132 | instead use strscpy_pad(). | |
84253c8b | 133 | |
053f8fc7 | 134 | If a caller is using non-NUL-terminated strings, strncpy() can |
84253c8b KC |
135 | still be used, but destinations should be marked with the `__nonstring |
136 | <https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html>`_ | |
137 | attribute to avoid future compiler warnings. | |
138 | ||
139 | strlcpy() | |
140 | --------- | |
27def953 KC |
141 | strlcpy() reads the entire source buffer first (since the return value |
142 | is meant to match that of strlen()). This read may exceed the destination | |
143 | size limit. This is both inefficient and can lead to linear read overflows | |
144 | if a source string is not NUL-terminated. The safe replacement is strscpy(), | |
145 | though care must be given to any cases where the return value of strlcpy() | |
146 | is used, since strscpy() will return negative errno values when it truncates. | |
84253c8b | 147 | |
d8401f50 KC |
148 | %p format specifier |
149 | ------------------- | |
150 | Traditionally, using "%p" in format strings would lead to regular address | |
151 | exposure flaws in dmesg, proc, sysfs, etc. Instead of leaving these to | |
152 | be exploitable, all "%p" uses in the kernel are being printed as a hashed | |
153 | value, rendering them unusable for addressing. New uses of "%p" should not | |
154 | be added to the kernel. For text addresses, using "%pS" is likely better, | |
155 | as it produces the more useful symbol name instead. For nearly everything | |
156 | else, just do not add "%p" at all. | |
157 | ||
158 | Paraphrasing Linus's current `guidance <https://lore.kernel.org/lkml/CA+55aFwQEd_d40g4mUCSsVRZzrFPUJt74vc6PPpb675hYNXcKw@mail.gmail.com/>`_: | |
159 | ||
160 | - If the hashed "%p" value is pointless, ask yourself whether the pointer | |
161 | itself is important. Maybe it should be removed entirely? | |
162 | - If you really think the true pointer value is important, why is some | |
163 | system state or user privilege level considered "special"? If you think | |
164 | you can justify it (in comments and commit log) well enough to stand | |
165 | up to Linus's scrutiny, maybe you can use "%px", along with making sure | |
166 | you have sensible permissions. | |
167 | ||
6ab0493d KC |
168 | If you are debugging something where "%p" hashing is causing problems, |
169 | you can temporarily boot with the debug flag "`no_hash_pointers | |
170 | <https://git.kernel.org/linus/5ead723a20e0447bc7db33dc3070b420e5f80aa6>`_". | |
d8401f50 | 171 | |
84253c8b KC |
172 | Variable Length Arrays (VLAs) |
173 | ----------------------------- | |
174 | Using stack VLAs produces much worse machine code than statically | |
175 | sized stack arrays. While these non-trivial `performance issues | |
176 | <https://git.kernel.org/linus/02361bc77888>`_ are reason enough to | |
177 | eliminate VLAs, they are also a security risk. Dynamic growth of a stack | |
178 | array may exceed the remaining memory in the stack segment. This could | |
179 | lead to a crash, possible overwriting sensitive contents at the end of the | |
180 | stack (when built without `CONFIG_THREAD_INFO_IN_TASK=y`), or overwriting | |
181 | memory adjacent to the stack (when built without `CONFIG_VMAP_STACK=y`) | |
a035d552 GS |
182 | |
183 | Implicit switch case fall-through | |
184 | --------------------------------- | |
76136e02 KC |
185 | The C language allows switch cases to fall through to the next case |
186 | when a "break" statement is missing at the end of a case. This, however, | |
187 | introduces ambiguity in the code, as it's not always clear if the missing | |
188 | break is intentional or a bug. For example, it's not obvious just from | |
189 | looking at the code if `STATE_ONE` is intentionally designed to fall | |
190 | through into `STATE_TWO`:: | |
191 | ||
192 | switch (value) { | |
193 | case STATE_ONE: | |
194 | do_something(); | |
195 | case STATE_TWO: | |
196 | do_other(); | |
197 | break; | |
198 | default: | |
199 | WARN("unknown state"); | |
200 | } | |
b9918bdc JP |
201 | |
202 | As there have been a long list of flaws `due to missing "break" statements | |
a035d552 | 203 | <https://cwe.mitre.org/data/definitions/484.html>`_, we no longer allow |
76136e02 KC |
204 | implicit fall-through. In order to identify intentional fall-through |
205 | cases, we have adopted a pseudo-keyword macro "fallthrough" which | |
206 | expands to gcc's extension `__attribute__((__fallthrough__)) | |
207 | <https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_. | |
208 | (When the C17/C18 `[[fallthrough]]` syntax is more commonly supported by | |
b9918bdc | 209 | C compilers, static analyzers, and IDEs, we can switch to using that syntax |
76136e02 | 210 | for the macro pseudo-keyword.) |
b9918bdc JP |
211 | |
212 | All switch/case blocks must end in one of: | |
213 | ||
76136e02 KC |
214 | * break; |
215 | * fallthrough; | |
216 | * continue; | |
217 | * goto <label>; | |
218 | * return [expression]; | |
68e4cd17 GS |
219 | |
220 | Zero-length and one-element arrays | |
221 | ---------------------------------- | |
222 | There is a regular need in the kernel to provide a way to declare having | |
223 | a dynamically sized set of trailing elements in a structure. Kernel code | |
224 | should always use `"flexible array members" <https://en.wikipedia.org/wiki/Flexible_array_member>`_ | |
225 | for these cases. The older style of one-element or zero-length arrays should | |
226 | no longer be used. | |
227 | ||
228 | In older C code, dynamically sized trailing elements were done by specifying | |
229 | a one-element array at the end of a structure:: | |
230 | ||
231 | struct something { | |
232 | size_t count; | |
233 | struct foo items[1]; | |
234 | }; | |
235 | ||
236 | This led to fragile size calculations via sizeof() (which would need to | |
237 | remove the size of the single trailing element to get a correct size of | |
238 | the "header"). A `GNU C extension <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_ | |
239 | was introduced to allow for zero-length arrays, to avoid these kinds of | |
240 | size problems:: | |
241 | ||
242 | struct something { | |
243 | size_t count; | |
244 | struct foo items[0]; | |
245 | }; | |
246 | ||
247 | But this led to other problems, and didn't solve some problems shared by | |
248 | both styles, like not being able to detect when such an array is accidentally | |
249 | being used _not_ at the end of a structure (which could happen directly, or | |
250 | when such a struct was in unions, structs of structs, etc). | |
251 | ||
252 | C99 introduced "flexible array members", which lacks a numeric size for | |
253 | the array declaration entirely:: | |
254 | ||
255 | struct something { | |
256 | size_t count; | |
257 | struct foo items[]; | |
258 | }; | |
259 | ||
260 | This is the way the kernel expects dynamically sized trailing elements | |
261 | to be declared. It allows the compiler to generate errors when the | |
262 | flexible array does not occur last in the structure, which helps to prevent | |
263 | some kind of `undefined behavior | |
264 | <https://git.kernel.org/linus/76497732932f15e7323dc805e8ea8dc11bb587cf>`_ | |
265 | bugs from being inadvertently introduced to the codebase. It also allows | |
266 | the compiler to correctly analyze array sizes (via sizeof(), | |
267 | `CONFIG_FORTIFY_SOURCE`, and `CONFIG_UBSAN_BOUNDS`). For instance, | |
268 | there is no mechanism that warns us that the following application of the | |
269 | sizeof() operator to a zero-length array always results in zero:: | |
270 | ||
271 | struct something { | |
272 | size_t count; | |
273 | struct foo items[0]; | |
274 | }; | |
275 | ||
276 | struct something *instance; | |
277 | ||
278 | instance = kmalloc(struct_size(instance, items, count), GFP_KERNEL); | |
279 | instance->count = count; | |
280 | ||
281 | size = sizeof(instance->items) * instance->count; | |
282 | memcpy(instance->items, source, size); | |
283 | ||
284 | At the last line of code above, ``size`` turns out to be ``zero``, when one might | |
285 | have thought it represents the total size in bytes of the dynamic memory recently | |
286 | allocated for the trailing array ``items``. Here are a couple examples of this | |
287 | issue: `link 1 | |
288 | <https://git.kernel.org/linus/f2cd32a443da694ac4e28fbf4ac6f9d5cc63a539>`_, | |
289 | `link 2 | |
290 | <https://git.kernel.org/linus/ab91c2a89f86be2898cee208d492816ec238b2cf>`_. | |
291 | Instead, `flexible array members have incomplete type, and so the sizeof() | |
292 | operator may not be applied <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, | |
293 | so any misuse of such operators will be immediately noticed at build time. | |
294 | ||
295 | With respect to one-element arrays, one has to be acutely aware that `such arrays | |
296 | occupy at least as much space as a single object of the type | |
297 | <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, | |
298 | hence they contribute to the size of the enclosing structure. This is prone | |
299 | to error every time people want to calculate the total size of dynamic memory | |
300 | to allocate for a structure containing an array of this kind as a member:: | |
301 | ||
302 | struct something { | |
303 | size_t count; | |
304 | struct foo items[1]; | |
305 | }; | |
306 | ||
307 | struct something *instance; | |
308 | ||
309 | instance = kmalloc(struct_size(instance, items, count - 1), GFP_KERNEL); | |
310 | instance->count = count; | |
311 | ||
312 | size = sizeof(instance->items) * instance->count; | |
313 | memcpy(instance->items, source, size); | |
314 | ||
315 | In the example above, we had to remember to calculate ``count - 1`` when using | |
316 | the struct_size() helper, otherwise we would have --unintentionally-- allocated | |
317 | memory for one too many ``items`` objects. The cleanest and least error-prone way | |
17dca050 GS |
318 | to implement this is through the use of a `flexible array member`, together with |
319 | struct_size() and flex_array_size() helpers:: | |
68e4cd17 GS |
320 | |
321 | struct something { | |
322 | size_t count; | |
323 | struct foo items[]; | |
324 | }; | |
325 | ||
326 | struct something *instance; | |
327 | ||
328 | instance = kmalloc(struct_size(instance, items, count), GFP_KERNEL); | |
329 | instance->count = count; | |
330 | ||
17dca050 | 331 | memcpy(instance->items, source, flex_array_size(instance, items, instance->count)); |