184253c8bSKees Cook.. SPDX-License-Identifier: GPL-2.0 284253c8bSKees Cook 398348577SFederico Vaga.. _deprecated: 498348577SFederico Vaga 584253c8bSKees Cook===================================================================== 684253c8bSKees CookDeprecated Interfaces, Language Features, Attributes, and Conventions 784253c8bSKees Cook===================================================================== 884253c8bSKees Cook 984253c8bSKees CookIn a perfect world, it would be possible to convert all instances of 1084253c8bSKees Cooksome deprecated API into the new API and entirely remove the old API in 1184253c8bSKees Cooka single development cycle. However, due to the size of the kernel, the 1284253c8bSKees Cookmaintainership hierarchy, and timing, it's not always feasible to do these 1384253c8bSKees Cookkinds of conversions at once. This means that new instances may sneak into 1484253c8bSKees Cookthe kernel while old ones are being removed, only making the amount of 1584253c8bSKees Cookwork to remove the API grow. In order to educate developers about what 1684253c8bSKees Cookhas been deprecated and why, this list has been created as a place to 1784253c8bSKees Cookpoint when uses of deprecated things are proposed for inclusion in the 1884253c8bSKees Cookkernel. 1984253c8bSKees Cook 2084253c8bSKees Cook__deprecated 2184253c8bSKees Cook------------ 2284253c8bSKees CookWhile this attribute does visually mark an interface as deprecated, 2384253c8bSKees Cookit `does not produce warnings during builds any more 2484253c8bSKees Cook<https://git.kernel.org/linus/771c035372a036f83353eef46dbb829780330234>`_ 2584253c8bSKees Cookbecause one of the standing goals of the kernel is to build without 2684253c8bSKees Cookwarnings and no one was actually doing anything to remove these deprecated 2784253c8bSKees Cookinterfaces. While using `__deprecated` is nice to note an old API in 2884253c8bSKees Cooka header file, it isn't the full solution. Such interfaces must either 2984253c8bSKees Cookbe fully removed from the kernel, or added to this file to discourage 3084253c8bSKees Cookothers from using them in the future. 3184253c8bSKees Cook 327af51678SKees CookBUG() and BUG_ON() 337af51678SKees Cook------------------ 347af51678SKees CookUse WARN() and WARN_ON() instead, and handle the "impossible" 357af51678SKees Cookerror condition as gracefully as possible. While the BUG()-family 367af51678SKees Cookof APIs were originally designed to act as an "impossible situation" 377af51678SKees Cookassert and to kill a kernel thread "safely", they turn out to just be 387af51678SKees Cooktoo risky. (e.g. "In what order do locks need to be released? Have 397af51678SKees Cookvarious states been restored?") Very commonly, using BUG() will 407af51678SKees Cookdestabilize a system or entirely break it, which makes it impossible 417af51678SKees Cookto debug or even get viable crash reports. Linus has `very strong 427af51678SKees Cook<https://lore.kernel.org/lkml/CA+55aFy6jNLsywVYdGp83AMrXBo_P-pkjkphPGrO=82SPKCpLQ@mail.gmail.com/>`_ 437af51678SKees Cookfeelings `about this 447af51678SKees Cook<https://lore.kernel.org/lkml/CAHk-=whDHsbK3HTOpTF=ue_o04onRwTEaK_ZoJp_fjbqq4+=Jw@mail.gmail.com/>`_. 457af51678SKees Cook 467af51678SKees CookNote that the WARN()-family should only be used for "expected to 477af51678SKees Cookbe unreachable" situations. If you want to warn about "reachable 487af51678SKees Cookbut undesirable" situations, please use the pr_warn()-family of 497af51678SKees Cookfunctions. System owners may have set the *panic_on_warn* sysctl, 507af51678SKees Cookto make sure their systems do not continue running in the face of 517af51678SKees Cook"unreachable" conditions. (For example, see commits like `this one 527af51678SKees Cook<https://git.kernel.org/linus/d4689846881d160a4d12a514e991a740bcb5d65a>`_.) 537af51678SKees Cook 5484253c8bSKees Cookopen-coded arithmetic in allocator arguments 5584253c8bSKees Cook-------------------------------------------- 5684253c8bSKees CookDynamic size calculations (especially multiplication) should not be 5784253c8bSKees Cookperformed in memory allocator (or similar) function arguments due to the 5884253c8bSKees Cookrisk of them overflowing. This could lead to values wrapping around and a 5984253c8bSKees Cooksmaller allocation being made than the caller was expecting. Using those 6084253c8bSKees Cookallocations could lead to linear overflows of heap memory and other 6184253c8bSKees Cookmisbehaviors. (One exception to this is literal values where the compiler 6284253c8bSKees Cookcan warn if they might overflow. Though using literals for arguments as 6384253c8bSKees Cooksuggested below is also harmless.) 6484253c8bSKees Cook 6584253c8bSKees CookFor example, do not use ``count * size`` as an argument, as in:: 6684253c8bSKees Cook 6784253c8bSKees Cook foo = kmalloc(count * size, GFP_KERNEL); 6884253c8bSKees Cook 6984253c8bSKees CookInstead, the 2-factor form of the allocator should be used:: 7084253c8bSKees Cook 7184253c8bSKees Cook foo = kmalloc_array(count, size, GFP_KERNEL); 7284253c8bSKees Cook 7384253c8bSKees CookIf no 2-factor form is available, the saturate-on-overflow helpers should 7484253c8bSKees Cookbe used:: 7584253c8bSKees Cook 7684253c8bSKees Cook bar = vmalloc(array_size(count, size)); 7784253c8bSKees Cook 7884253c8bSKees CookAnother common case to avoid is calculating the size of a structure with 7984253c8bSKees Cooka trailing array of others structures, as in:: 8084253c8bSKees Cook 8184253c8bSKees Cook header = kzalloc(sizeof(*header) + count * sizeof(*header->item), 8284253c8bSKees Cook GFP_KERNEL); 8384253c8bSKees Cook 8484253c8bSKees CookInstead, use the helper:: 8584253c8bSKees Cook 8684253c8bSKees Cook header = kzalloc(struct_size(header, item, count), GFP_KERNEL); 8784253c8bSKees Cook 8868e4cd17SGustavo A. R. Silva.. note:: If you are using struct_size() on a structure containing a zero-length 8968e4cd17SGustavo A. R. Silva or a one-element array as a trailing array member, please refactor such 9068e4cd17SGustavo A. R. Silva array usage and switch to a `flexible array member 9168e4cd17SGustavo A. R. Silva <#zero-length-and-one-element-arrays>`_ instead. 9268e4cd17SGustavo A. R. Silva 937929b983SJonathan CorbetSee array_size(), array3_size(), and struct_size(), 947929b983SJonathan Corbetfor more details as well as the related check_add_overflow() and 957929b983SJonathan Corbetcheck_mul_overflow() family of functions. 9684253c8bSKees Cook 9784253c8bSKees Cooksimple_strtol(), simple_strtoll(), simple_strtoul(), simple_strtoull() 9884253c8bSKees Cook---------------------------------------------------------------------- 997929b983SJonathan CorbetThe simple_strtol(), simple_strtoll(), 1007929b983SJonathan Corbetsimple_strtoul(), and simple_strtoull() functions 10184253c8bSKees Cookexplicitly ignore overflows, which may lead to unexpected results 1027929b983SJonathan Corbetin callers. The respective kstrtol(), kstrtoll(), 1037929b983SJonathan Corbetkstrtoul(), and kstrtoull() functions tend to be the 10484253c8bSKees Cookcorrect replacements, though note that those require the string to be 10584253c8bSKees CookNUL or newline terminated. 10684253c8bSKees Cook 10784253c8bSKees Cookstrcpy() 10884253c8bSKees Cook-------- 1097929b983SJonathan Corbetstrcpy() performs no bounds checking on the destination 11084253c8bSKees Cookbuffer. This could result in linear overflows beyond the 11184253c8bSKees Cookend of the buffer, leading to all kinds of misbehaviors. While 11284253c8bSKees Cook`CONFIG_FORTIFY_SOURCE=y` and various compiler flags help reduce the 11384253c8bSKees Cookrisk of using this function, there is no good reason to add new uses of 1147929b983SJonathan Corbetthis function. The safe replacement is strscpy(). 11584253c8bSKees Cook 11684253c8bSKees Cookstrncpy() on NUL-terminated strings 11784253c8bSKees Cook----------------------------------- 1187929b983SJonathan CorbetUse of strncpy() does not guarantee that the destination buffer 11984253c8bSKees Cookwill be NUL terminated. This can lead to various linear read overflows 12084253c8bSKees Cookand other misbehavior due to the missing termination. It also NUL-pads the 12184253c8bSKees Cookdestination buffer if the source contents are shorter than the destination 12284253c8bSKees Cookbuffer size, which may be a needless performance penalty for callers using 1237929b983SJonathan Corbetonly NUL-terminated strings. The safe replacement is strscpy(). 1247929b983SJonathan Corbet(Users of strscpy() still needing NUL-padding should instead 12576136e02SKees Cookuse strscpy_pad().) 12684253c8bSKees Cook 127053f8fc7SKees CookIf a caller is using non-NUL-terminated strings, strncpy() can 12884253c8bSKees Cookstill be used, but destinations should be marked with the `__nonstring 12984253c8bSKees Cook<https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html>`_ 13084253c8bSKees Cookattribute to avoid future compiler warnings. 13184253c8bSKees Cook 13284253c8bSKees Cookstrlcpy() 13384253c8bSKees Cook--------- 1347929b983SJonathan Corbetstrlcpy() reads the entire source buffer first, possibly exceeding 13584253c8bSKees Cookthe given limit of bytes to copy. This is inefficient and can lead to 13684253c8bSKees Cooklinear read overflows if a source string is not NUL-terminated. The 1377929b983SJonathan Corbetsafe replacement is strscpy(). 13884253c8bSKees Cook 139d8401f50SKees Cook%p format specifier 140d8401f50SKees Cook------------------- 141d8401f50SKees CookTraditionally, using "%p" in format strings would lead to regular address 142d8401f50SKees Cookexposure flaws in dmesg, proc, sysfs, etc. Instead of leaving these to 143d8401f50SKees Cookbe exploitable, all "%p" uses in the kernel are being printed as a hashed 144d8401f50SKees Cookvalue, rendering them unusable for addressing. New uses of "%p" should not 145d8401f50SKees Cookbe added to the kernel. For text addresses, using "%pS" is likely better, 146d8401f50SKees Cookas it produces the more useful symbol name instead. For nearly everything 147d8401f50SKees Cookelse, just do not add "%p" at all. 148d8401f50SKees Cook 149d8401f50SKees CookParaphrasing Linus's current `guidance <https://lore.kernel.org/lkml/CA+55aFwQEd_d40g4mUCSsVRZzrFPUJt74vc6PPpb675hYNXcKw@mail.gmail.com/>`_: 150d8401f50SKees Cook 151d8401f50SKees Cook- If the hashed "%p" value is pointless, ask yourself whether the pointer 152d8401f50SKees Cook itself is important. Maybe it should be removed entirely? 153d8401f50SKees Cook- If you really think the true pointer value is important, why is some 154d8401f50SKees Cook system state or user privilege level considered "special"? If you think 155d8401f50SKees Cook you can justify it (in comments and commit log) well enough to stand 156d8401f50SKees Cook up to Linus's scrutiny, maybe you can use "%px", along with making sure 157d8401f50SKees Cook you have sensible permissions. 158d8401f50SKees Cook 159d8401f50SKees CookAnd finally, know that a toggle for "%p" hashing will `not be accepted <https://lore.kernel.org/lkml/CA+55aFwieC1-nAs+NFq9RTwaR8ef9hWa4MjNBWL41F-8wM49eA@mail.gmail.com/>`_. 160d8401f50SKees Cook 16184253c8bSKees CookVariable Length Arrays (VLAs) 16284253c8bSKees Cook----------------------------- 16384253c8bSKees CookUsing stack VLAs produces much worse machine code than statically 16484253c8bSKees Cooksized stack arrays. While these non-trivial `performance issues 16584253c8bSKees Cook<https://git.kernel.org/linus/02361bc77888>`_ are reason enough to 16684253c8bSKees Cookeliminate VLAs, they are also a security risk. Dynamic growth of a stack 16784253c8bSKees Cookarray may exceed the remaining memory in the stack segment. This could 16884253c8bSKees Cooklead to a crash, possible overwriting sensitive contents at the end of the 16984253c8bSKees Cookstack (when built without `CONFIG_THREAD_INFO_IN_TASK=y`), or overwriting 17084253c8bSKees Cookmemory adjacent to the stack (when built without `CONFIG_VMAP_STACK=y`) 171a035d552SGustavo A. R. Silva 172a035d552SGustavo A. R. SilvaImplicit switch case fall-through 173a035d552SGustavo A. R. Silva--------------------------------- 17476136e02SKees CookThe C language allows switch cases to fall through to the next case 17576136e02SKees Cookwhen a "break" statement is missing at the end of a case. This, however, 17676136e02SKees Cookintroduces ambiguity in the code, as it's not always clear if the missing 17776136e02SKees Cookbreak is intentional or a bug. For example, it's not obvious just from 17876136e02SKees Cooklooking at the code if `STATE_ONE` is intentionally designed to fall 17976136e02SKees Cookthrough into `STATE_TWO`:: 18076136e02SKees Cook 18176136e02SKees Cook switch (value) { 18276136e02SKees Cook case STATE_ONE: 18376136e02SKees Cook do_something(); 18476136e02SKees Cook case STATE_TWO: 18576136e02SKees Cook do_other(); 18676136e02SKees Cook break; 18776136e02SKees Cook default: 18876136e02SKees Cook WARN("unknown state"); 18976136e02SKees Cook } 190b9918bdcSJoe Perches 191b9918bdcSJoe PerchesAs there have been a long list of flaws `due to missing "break" statements 192a035d552SGustavo A. R. Silva<https://cwe.mitre.org/data/definitions/484.html>`_, we no longer allow 19376136e02SKees Cookimplicit fall-through. In order to identify intentional fall-through 19476136e02SKees Cookcases, we have adopted a pseudo-keyword macro "fallthrough" which 19576136e02SKees Cookexpands to gcc's extension `__attribute__((__fallthrough__)) 19676136e02SKees Cook<https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_. 19776136e02SKees Cook(When the C17/C18 `[[fallthrough]]` syntax is more commonly supported by 198b9918bdcSJoe PerchesC compilers, static analyzers, and IDEs, we can switch to using that syntax 19976136e02SKees Cookfor the macro pseudo-keyword.) 200b9918bdcSJoe Perches 201b9918bdcSJoe PerchesAll switch/case blocks must end in one of: 202b9918bdcSJoe Perches 20376136e02SKees Cook* break; 20476136e02SKees Cook* fallthrough; 20576136e02SKees Cook* continue; 20676136e02SKees Cook* goto <label>; 20776136e02SKees Cook* return [expression]; 20868e4cd17SGustavo A. R. Silva 20968e4cd17SGustavo A. R. SilvaZero-length and one-element arrays 21068e4cd17SGustavo A. R. Silva---------------------------------- 21168e4cd17SGustavo A. R. SilvaThere is a regular need in the kernel to provide a way to declare having 21268e4cd17SGustavo A. R. Silvaa dynamically sized set of trailing elements in a structure. Kernel code 21368e4cd17SGustavo A. R. Silvashould always use `"flexible array members" <https://en.wikipedia.org/wiki/Flexible_array_member>`_ 21468e4cd17SGustavo A. R. Silvafor these cases. The older style of one-element or zero-length arrays should 21568e4cd17SGustavo A. R. Silvano longer be used. 21668e4cd17SGustavo A. R. Silva 21768e4cd17SGustavo A. R. SilvaIn older C code, dynamically sized trailing elements were done by specifying 21868e4cd17SGustavo A. R. Silvaa one-element array at the end of a structure:: 21968e4cd17SGustavo A. R. Silva 22068e4cd17SGustavo A. R. Silva struct something { 22168e4cd17SGustavo A. R. Silva size_t count; 22268e4cd17SGustavo A. R. Silva struct foo items[1]; 22368e4cd17SGustavo A. R. Silva }; 22468e4cd17SGustavo A. R. Silva 22568e4cd17SGustavo A. R. SilvaThis led to fragile size calculations via sizeof() (which would need to 22668e4cd17SGustavo A. R. Silvaremove the size of the single trailing element to get a correct size of 22768e4cd17SGustavo A. R. Silvathe "header"). A `GNU C extension <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_ 22868e4cd17SGustavo A. R. Silvawas introduced to allow for zero-length arrays, to avoid these kinds of 22968e4cd17SGustavo A. R. Silvasize problems:: 23068e4cd17SGustavo A. R. Silva 23168e4cd17SGustavo A. R. Silva struct something { 23268e4cd17SGustavo A. R. Silva size_t count; 23368e4cd17SGustavo A. R. Silva struct foo items[0]; 23468e4cd17SGustavo A. R. Silva }; 23568e4cd17SGustavo A. R. Silva 23668e4cd17SGustavo A. R. SilvaBut this led to other problems, and didn't solve some problems shared by 23768e4cd17SGustavo A. R. Silvaboth styles, like not being able to detect when such an array is accidentally 23868e4cd17SGustavo A. R. Silvabeing used _not_ at the end of a structure (which could happen directly, or 23968e4cd17SGustavo A. R. Silvawhen such a struct was in unions, structs of structs, etc). 24068e4cd17SGustavo A. R. Silva 24168e4cd17SGustavo A. R. SilvaC99 introduced "flexible array members", which lacks a numeric size for 24268e4cd17SGustavo A. R. Silvathe array declaration entirely:: 24368e4cd17SGustavo A. R. Silva 24468e4cd17SGustavo A. R. Silva struct something { 24568e4cd17SGustavo A. R. Silva size_t count; 24668e4cd17SGustavo A. R. Silva struct foo items[]; 24768e4cd17SGustavo A. R. Silva }; 24868e4cd17SGustavo A. R. Silva 24968e4cd17SGustavo A. R. SilvaThis is the way the kernel expects dynamically sized trailing elements 25068e4cd17SGustavo A. R. Silvato be declared. It allows the compiler to generate errors when the 25168e4cd17SGustavo A. R. Silvaflexible array does not occur last in the structure, which helps to prevent 25268e4cd17SGustavo A. R. Silvasome kind of `undefined behavior 25368e4cd17SGustavo A. R. Silva<https://git.kernel.org/linus/76497732932f15e7323dc805e8ea8dc11bb587cf>`_ 25468e4cd17SGustavo A. R. Silvabugs from being inadvertently introduced to the codebase. It also allows 25568e4cd17SGustavo A. R. Silvathe compiler to correctly analyze array sizes (via sizeof(), 25668e4cd17SGustavo A. R. Silva`CONFIG_FORTIFY_SOURCE`, and `CONFIG_UBSAN_BOUNDS`). For instance, 25768e4cd17SGustavo A. R. Silvathere is no mechanism that warns us that the following application of the 25868e4cd17SGustavo A. R. Silvasizeof() operator to a zero-length array always results in zero:: 25968e4cd17SGustavo A. R. Silva 26068e4cd17SGustavo A. R. Silva struct something { 26168e4cd17SGustavo A. R. Silva size_t count; 26268e4cd17SGustavo A. R. Silva struct foo items[0]; 26368e4cd17SGustavo A. R. Silva }; 26468e4cd17SGustavo A. R. Silva 26568e4cd17SGustavo A. R. Silva struct something *instance; 26668e4cd17SGustavo A. R. Silva 26768e4cd17SGustavo A. R. Silva instance = kmalloc(struct_size(instance, items, count), GFP_KERNEL); 26868e4cd17SGustavo A. R. Silva instance->count = count; 26968e4cd17SGustavo A. R. Silva 27068e4cd17SGustavo A. R. Silva size = sizeof(instance->items) * instance->count; 27168e4cd17SGustavo A. R. Silva memcpy(instance->items, source, size); 27268e4cd17SGustavo A. R. Silva 27368e4cd17SGustavo A. R. SilvaAt the last line of code above, ``size`` turns out to be ``zero``, when one might 27468e4cd17SGustavo A. R. Silvahave thought it represents the total size in bytes of the dynamic memory recently 27568e4cd17SGustavo A. R. Silvaallocated for the trailing array ``items``. Here are a couple examples of this 27668e4cd17SGustavo A. R. Silvaissue: `link 1 27768e4cd17SGustavo A. R. Silva<https://git.kernel.org/linus/f2cd32a443da694ac4e28fbf4ac6f9d5cc63a539>`_, 27868e4cd17SGustavo A. R. Silva`link 2 27968e4cd17SGustavo A. R. Silva<https://git.kernel.org/linus/ab91c2a89f86be2898cee208d492816ec238b2cf>`_. 28068e4cd17SGustavo A. R. SilvaInstead, `flexible array members have incomplete type, and so the sizeof() 28168e4cd17SGustavo A. R. Silvaoperator may not be applied <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, 28268e4cd17SGustavo A. R. Silvaso any misuse of such operators will be immediately noticed at build time. 28368e4cd17SGustavo A. R. Silva 28468e4cd17SGustavo A. R. SilvaWith respect to one-element arrays, one has to be acutely aware that `such arrays 28568e4cd17SGustavo A. R. Silvaoccupy at least as much space as a single object of the type 28668e4cd17SGustavo A. R. Silva<https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, 28768e4cd17SGustavo A. R. Silvahence they contribute to the size of the enclosing structure. This is prone 28868e4cd17SGustavo A. R. Silvato error every time people want to calculate the total size of dynamic memory 28968e4cd17SGustavo A. R. Silvato allocate for a structure containing an array of this kind as a member:: 29068e4cd17SGustavo A. R. Silva 29168e4cd17SGustavo A. R. Silva struct something { 29268e4cd17SGustavo A. R. Silva size_t count; 29368e4cd17SGustavo A. R. Silva struct foo items[1]; 29468e4cd17SGustavo A. R. Silva }; 29568e4cd17SGustavo A. R. Silva 29668e4cd17SGustavo A. R. Silva struct something *instance; 29768e4cd17SGustavo A. R. Silva 29868e4cd17SGustavo A. R. Silva instance = kmalloc(struct_size(instance, items, count - 1), GFP_KERNEL); 29968e4cd17SGustavo A. R. Silva instance->count = count; 30068e4cd17SGustavo A. R. Silva 30168e4cd17SGustavo A. R. Silva size = sizeof(instance->items) * instance->count; 30268e4cd17SGustavo A. R. Silva memcpy(instance->items, source, size); 30368e4cd17SGustavo A. R. Silva 30468e4cd17SGustavo A. R. SilvaIn the example above, we had to remember to calculate ``count - 1`` when using 30568e4cd17SGustavo A. R. Silvathe struct_size() helper, otherwise we would have --unintentionally-- allocated 30668e4cd17SGustavo A. R. Silvamemory for one too many ``items`` objects. The cleanest and least error-prone way 307*17dca050SGustavo A. R. Silvato implement this is through the use of a `flexible array member`, together with 308*17dca050SGustavo A. R. Silvastruct_size() and flex_array_size() helpers:: 30968e4cd17SGustavo A. R. Silva 31068e4cd17SGustavo A. R. Silva struct something { 31168e4cd17SGustavo A. R. Silva size_t count; 31268e4cd17SGustavo A. R. Silva struct foo items[]; 31368e4cd17SGustavo A. R. Silva }; 31468e4cd17SGustavo A. R. Silva 31568e4cd17SGustavo A. R. Silva struct something *instance; 31668e4cd17SGustavo A. R. Silva 31768e4cd17SGustavo A. R. Silva instance = kmalloc(struct_size(instance, items, count), GFP_KERNEL); 31868e4cd17SGustavo A. R. Silva instance->count = count; 31968e4cd17SGustavo A. R. Silva 320*17dca050SGustavo A. R. Silva memcpy(instance->items, source, flex_array_size(instance, items, instance->count)); 321