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-------- 10927def953SKees Cookstrcpy() performs no bounds checking on the destination buffer. This 11027def953SKees Cookcould result in linear overflows beyond the end of the buffer, leading to 11127def953SKees Cookall kinds of misbehaviors. While `CONFIG_FORTIFY_SOURCE=y` and various 11227def953SKees Cookcompiler flags help reduce the risk of using this function, there is 11327def953SKees Cookno good reason to add new uses of this function. The safe replacement 11427def953SKees Cookis strscpy(), though care must be given to any cases where the return 11527def953SKees Cookvalue of strcpy() was used, since strscpy() does not return a pointer to 11627def953SKees Cookthe destination, but rather a count of non-NUL bytes copied (or negative 11727def953SKees Cookerrno when it truncates). 11884253c8bSKees Cook 11984253c8bSKees Cookstrncpy() on NUL-terminated strings 12084253c8bSKees Cook----------------------------------- 12127def953SKees CookUse of strncpy() does not guarantee that the destination buffer will 12227def953SKees Cookbe NUL terminated. This can lead to various linear read overflows and 12327def953SKees Cookother misbehavior due to the missing termination. It also NUL-pads 12427def953SKees Cookthe destination buffer if the source contents are shorter than the 12527def953SKees Cookdestination buffer size, which may be a needless performance penalty 12627def953SKees Cookfor callers using only NUL-terminated strings. The safe replacement is 12727def953SKees Cookstrscpy(), though care must be given to any cases where the return value 12827def953SKees Cookof strncpy() was used, since strscpy() does not return a pointer to the 12927def953SKees Cookdestination, but rather a count of non-NUL bytes copied (or negative 13027def953SKees Cookerrno when it truncates). Any cases still needing NUL-padding should 13127def953SKees Cookinstead use strscpy_pad(). 13284253c8bSKees Cook 133053f8fc7SKees CookIf a caller is using non-NUL-terminated strings, strncpy() can 13484253c8bSKees Cookstill be used, but destinations should be marked with the `__nonstring 13584253c8bSKees Cook<https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html>`_ 13684253c8bSKees Cookattribute to avoid future compiler warnings. 13784253c8bSKees Cook 13884253c8bSKees Cookstrlcpy() 13984253c8bSKees Cook--------- 14027def953SKees Cookstrlcpy() reads the entire source buffer first (since the return value 14127def953SKees Cookis meant to match that of strlen()). This read may exceed the destination 14227def953SKees Cooksize limit. This is both inefficient and can lead to linear read overflows 14327def953SKees Cookif a source string is not NUL-terminated. The safe replacement is strscpy(), 14427def953SKees Cookthough care must be given to any cases where the return value of strlcpy() 14527def953SKees Cookis used, since strscpy() will return negative errno values when it truncates. 14684253c8bSKees Cook 147d8401f50SKees Cook%p format specifier 148d8401f50SKees Cook------------------- 149d8401f50SKees CookTraditionally, using "%p" in format strings would lead to regular address 150d8401f50SKees Cookexposure flaws in dmesg, proc, sysfs, etc. Instead of leaving these to 151d8401f50SKees Cookbe exploitable, all "%p" uses in the kernel are being printed as a hashed 152d8401f50SKees Cookvalue, rendering them unusable for addressing. New uses of "%p" should not 153d8401f50SKees Cookbe added to the kernel. For text addresses, using "%pS" is likely better, 154d8401f50SKees Cookas it produces the more useful symbol name instead. For nearly everything 155d8401f50SKees Cookelse, just do not add "%p" at all. 156d8401f50SKees Cook 157d8401f50SKees CookParaphrasing Linus's current `guidance <https://lore.kernel.org/lkml/CA+55aFwQEd_d40g4mUCSsVRZzrFPUJt74vc6PPpb675hYNXcKw@mail.gmail.com/>`_: 158d8401f50SKees Cook 159d8401f50SKees Cook- If the hashed "%p" value is pointless, ask yourself whether the pointer 160d8401f50SKees Cook itself is important. Maybe it should be removed entirely? 161d8401f50SKees Cook- If you really think the true pointer value is important, why is some 162d8401f50SKees Cook system state or user privilege level considered "special"? If you think 163d8401f50SKees Cook you can justify it (in comments and commit log) well enough to stand 164d8401f50SKees Cook up to Linus's scrutiny, maybe you can use "%px", along with making sure 165d8401f50SKees Cook you have sensible permissions. 166d8401f50SKees Cook 167*6ab0493dSKees CookIf you are debugging something where "%p" hashing is causing problems, 168*6ab0493dSKees Cookyou can temporarily boot with the debug flag "`no_hash_pointers 169*6ab0493dSKees Cook<https://git.kernel.org/linus/5ead723a20e0447bc7db33dc3070b420e5f80aa6>`_". 170d8401f50SKees Cook 17184253c8bSKees CookVariable Length Arrays (VLAs) 17284253c8bSKees Cook----------------------------- 17384253c8bSKees CookUsing stack VLAs produces much worse machine code than statically 17484253c8bSKees Cooksized stack arrays. While these non-trivial `performance issues 17584253c8bSKees Cook<https://git.kernel.org/linus/02361bc77888>`_ are reason enough to 17684253c8bSKees Cookeliminate VLAs, they are also a security risk. Dynamic growth of a stack 17784253c8bSKees Cookarray may exceed the remaining memory in the stack segment. This could 17884253c8bSKees Cooklead to a crash, possible overwriting sensitive contents at the end of the 17984253c8bSKees Cookstack (when built without `CONFIG_THREAD_INFO_IN_TASK=y`), or overwriting 18084253c8bSKees Cookmemory adjacent to the stack (when built without `CONFIG_VMAP_STACK=y`) 181a035d552SGustavo A. R. Silva 182a035d552SGustavo A. R. SilvaImplicit switch case fall-through 183a035d552SGustavo A. R. Silva--------------------------------- 18476136e02SKees CookThe C language allows switch cases to fall through to the next case 18576136e02SKees Cookwhen a "break" statement is missing at the end of a case. This, however, 18676136e02SKees Cookintroduces ambiguity in the code, as it's not always clear if the missing 18776136e02SKees Cookbreak is intentional or a bug. For example, it's not obvious just from 18876136e02SKees Cooklooking at the code if `STATE_ONE` is intentionally designed to fall 18976136e02SKees Cookthrough into `STATE_TWO`:: 19076136e02SKees Cook 19176136e02SKees Cook switch (value) { 19276136e02SKees Cook case STATE_ONE: 19376136e02SKees Cook do_something(); 19476136e02SKees Cook case STATE_TWO: 19576136e02SKees Cook do_other(); 19676136e02SKees Cook break; 19776136e02SKees Cook default: 19876136e02SKees Cook WARN("unknown state"); 19976136e02SKees Cook } 200b9918bdcSJoe Perches 201b9918bdcSJoe PerchesAs there have been a long list of flaws `due to missing "break" statements 202a035d552SGustavo A. R. Silva<https://cwe.mitre.org/data/definitions/484.html>`_, we no longer allow 20376136e02SKees Cookimplicit fall-through. In order to identify intentional fall-through 20476136e02SKees Cookcases, we have adopted a pseudo-keyword macro "fallthrough" which 20576136e02SKees Cookexpands to gcc's extension `__attribute__((__fallthrough__)) 20676136e02SKees Cook<https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_. 20776136e02SKees Cook(When the C17/C18 `[[fallthrough]]` syntax is more commonly supported by 208b9918bdcSJoe PerchesC compilers, static analyzers, and IDEs, we can switch to using that syntax 20976136e02SKees Cookfor the macro pseudo-keyword.) 210b9918bdcSJoe Perches 211b9918bdcSJoe PerchesAll switch/case blocks must end in one of: 212b9918bdcSJoe Perches 21376136e02SKees Cook* break; 21476136e02SKees Cook* fallthrough; 21576136e02SKees Cook* continue; 21676136e02SKees Cook* goto <label>; 21776136e02SKees Cook* return [expression]; 21868e4cd17SGustavo A. R. Silva 21968e4cd17SGustavo A. R. SilvaZero-length and one-element arrays 22068e4cd17SGustavo A. R. Silva---------------------------------- 22168e4cd17SGustavo A. R. SilvaThere is a regular need in the kernel to provide a way to declare having 22268e4cd17SGustavo A. R. Silvaa dynamically sized set of trailing elements in a structure. Kernel code 22368e4cd17SGustavo A. R. Silvashould always use `"flexible array members" <https://en.wikipedia.org/wiki/Flexible_array_member>`_ 22468e4cd17SGustavo A. R. Silvafor these cases. The older style of one-element or zero-length arrays should 22568e4cd17SGustavo A. R. Silvano longer be used. 22668e4cd17SGustavo A. R. Silva 22768e4cd17SGustavo A. R. SilvaIn older C code, dynamically sized trailing elements were done by specifying 22868e4cd17SGustavo A. R. Silvaa one-element array at the end of a structure:: 22968e4cd17SGustavo A. R. Silva 23068e4cd17SGustavo A. R. Silva struct something { 23168e4cd17SGustavo A. R. Silva size_t count; 23268e4cd17SGustavo A. R. Silva struct foo items[1]; 23368e4cd17SGustavo A. R. Silva }; 23468e4cd17SGustavo A. R. Silva 23568e4cd17SGustavo A. R. SilvaThis led to fragile size calculations via sizeof() (which would need to 23668e4cd17SGustavo A. R. Silvaremove the size of the single trailing element to get a correct size of 23768e4cd17SGustavo A. R. Silvathe "header"). A `GNU C extension <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_ 23868e4cd17SGustavo A. R. Silvawas introduced to allow for zero-length arrays, to avoid these kinds of 23968e4cd17SGustavo A. R. Silvasize problems:: 24068e4cd17SGustavo A. R. Silva 24168e4cd17SGustavo A. R. Silva struct something { 24268e4cd17SGustavo A. R. Silva size_t count; 24368e4cd17SGustavo A. R. Silva struct foo items[0]; 24468e4cd17SGustavo A. R. Silva }; 24568e4cd17SGustavo A. R. Silva 24668e4cd17SGustavo A. R. SilvaBut this led to other problems, and didn't solve some problems shared by 24768e4cd17SGustavo A. R. Silvaboth styles, like not being able to detect when such an array is accidentally 24868e4cd17SGustavo A. R. Silvabeing used _not_ at the end of a structure (which could happen directly, or 24968e4cd17SGustavo A. R. Silvawhen such a struct was in unions, structs of structs, etc). 25068e4cd17SGustavo A. R. Silva 25168e4cd17SGustavo A. R. SilvaC99 introduced "flexible array members", which lacks a numeric size for 25268e4cd17SGustavo A. R. Silvathe array declaration entirely:: 25368e4cd17SGustavo A. R. Silva 25468e4cd17SGustavo A. R. Silva struct something { 25568e4cd17SGustavo A. R. Silva size_t count; 25668e4cd17SGustavo A. R. Silva struct foo items[]; 25768e4cd17SGustavo A. R. Silva }; 25868e4cd17SGustavo A. R. Silva 25968e4cd17SGustavo A. R. SilvaThis is the way the kernel expects dynamically sized trailing elements 26068e4cd17SGustavo A. R. Silvato be declared. It allows the compiler to generate errors when the 26168e4cd17SGustavo A. R. Silvaflexible array does not occur last in the structure, which helps to prevent 26268e4cd17SGustavo A. R. Silvasome kind of `undefined behavior 26368e4cd17SGustavo A. R. Silva<https://git.kernel.org/linus/76497732932f15e7323dc805e8ea8dc11bb587cf>`_ 26468e4cd17SGustavo A. R. Silvabugs from being inadvertently introduced to the codebase. It also allows 26568e4cd17SGustavo A. R. Silvathe compiler to correctly analyze array sizes (via sizeof(), 26668e4cd17SGustavo A. R. Silva`CONFIG_FORTIFY_SOURCE`, and `CONFIG_UBSAN_BOUNDS`). For instance, 26768e4cd17SGustavo A. R. Silvathere is no mechanism that warns us that the following application of the 26868e4cd17SGustavo A. R. Silvasizeof() operator to a zero-length array always results in zero:: 26968e4cd17SGustavo A. R. Silva 27068e4cd17SGustavo A. R. Silva struct something { 27168e4cd17SGustavo A. R. Silva size_t count; 27268e4cd17SGustavo A. R. Silva struct foo items[0]; 27368e4cd17SGustavo A. R. Silva }; 27468e4cd17SGustavo A. R. Silva 27568e4cd17SGustavo A. R. Silva struct something *instance; 27668e4cd17SGustavo A. R. Silva 27768e4cd17SGustavo A. R. Silva instance = kmalloc(struct_size(instance, items, count), GFP_KERNEL); 27868e4cd17SGustavo A. R. Silva instance->count = count; 27968e4cd17SGustavo A. R. Silva 28068e4cd17SGustavo A. R. Silva size = sizeof(instance->items) * instance->count; 28168e4cd17SGustavo A. R. Silva memcpy(instance->items, source, size); 28268e4cd17SGustavo A. R. Silva 28368e4cd17SGustavo A. R. SilvaAt the last line of code above, ``size`` turns out to be ``zero``, when one might 28468e4cd17SGustavo A. R. Silvahave thought it represents the total size in bytes of the dynamic memory recently 28568e4cd17SGustavo A. R. Silvaallocated for the trailing array ``items``. Here are a couple examples of this 28668e4cd17SGustavo A. R. Silvaissue: `link 1 28768e4cd17SGustavo A. R. Silva<https://git.kernel.org/linus/f2cd32a443da694ac4e28fbf4ac6f9d5cc63a539>`_, 28868e4cd17SGustavo A. R. Silva`link 2 28968e4cd17SGustavo A. R. Silva<https://git.kernel.org/linus/ab91c2a89f86be2898cee208d492816ec238b2cf>`_. 29068e4cd17SGustavo A. R. SilvaInstead, `flexible array members have incomplete type, and so the sizeof() 29168e4cd17SGustavo A. R. Silvaoperator may not be applied <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, 29268e4cd17SGustavo A. R. Silvaso any misuse of such operators will be immediately noticed at build time. 29368e4cd17SGustavo A. R. Silva 29468e4cd17SGustavo A. R. SilvaWith respect to one-element arrays, one has to be acutely aware that `such arrays 29568e4cd17SGustavo A. R. Silvaoccupy at least as much space as a single object of the type 29668e4cd17SGustavo A. R. Silva<https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, 29768e4cd17SGustavo A. R. Silvahence they contribute to the size of the enclosing structure. This is prone 29868e4cd17SGustavo A. R. Silvato error every time people want to calculate the total size of dynamic memory 29968e4cd17SGustavo A. R. Silvato allocate for a structure containing an array of this kind as a member:: 30068e4cd17SGustavo A. R. Silva 30168e4cd17SGustavo A. R. Silva struct something { 30268e4cd17SGustavo A. R. Silva size_t count; 30368e4cd17SGustavo A. R. Silva struct foo items[1]; 30468e4cd17SGustavo A. R. Silva }; 30568e4cd17SGustavo A. R. Silva 30668e4cd17SGustavo A. R. Silva struct something *instance; 30768e4cd17SGustavo A. R. Silva 30868e4cd17SGustavo A. R. Silva instance = kmalloc(struct_size(instance, items, count - 1), GFP_KERNEL); 30968e4cd17SGustavo A. R. Silva instance->count = count; 31068e4cd17SGustavo A. R. Silva 31168e4cd17SGustavo A. R. Silva size = sizeof(instance->items) * instance->count; 31268e4cd17SGustavo A. R. Silva memcpy(instance->items, source, size); 31368e4cd17SGustavo A. R. Silva 31468e4cd17SGustavo A. R. SilvaIn the example above, we had to remember to calculate ``count - 1`` when using 31568e4cd17SGustavo A. R. Silvathe struct_size() helper, otherwise we would have --unintentionally-- allocated 31668e4cd17SGustavo A. R. Silvamemory for one too many ``items`` objects. The cleanest and least error-prone way 31717dca050SGustavo A. R. Silvato implement this is through the use of a `flexible array member`, together with 31817dca050SGustavo A. R. Silvastruct_size() and flex_array_size() helpers:: 31968e4cd17SGustavo A. R. Silva 32068e4cd17SGustavo A. R. Silva struct something { 32168e4cd17SGustavo A. R. Silva size_t count; 32268e4cd17SGustavo A. R. Silva struct foo items[]; 32368e4cd17SGustavo A. R. Silva }; 32468e4cd17SGustavo A. R. Silva 32568e4cd17SGustavo A. R. Silva struct something *instance; 32668e4cd17SGustavo A. R. Silva 32768e4cd17SGustavo A. R. Silva instance = kmalloc(struct_size(instance, items, count), GFP_KERNEL); 32868e4cd17SGustavo A. R. Silva instance->count = count; 32968e4cd17SGustavo A. R. Silva 33017dca050SGustavo A. R. Silva memcpy(instance->items, source, flex_array_size(instance, items, instance->count)); 331