From nobody Sun Feb 8 11:41:11 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9AB0C001B0 for ; Wed, 9 Aug 2023 03:26:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230339AbjHID0v (ORCPT ); Tue, 8 Aug 2023 23:26:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60986 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230218AbjHID0s (ORCPT ); Tue, 8 Aug 2023 23:26:48 -0400 Received: from sgoci-sdnproxy-4.icoremail.net (sgoci-sdnproxy-4.icoremail.net [129.150.39.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 2FC511999; Tue, 8 Aug 2023 20:26:43 -0700 (PDT) Received: from localhost.localdomain (unknown [39.174.92.167]) by mail-app3 (Coremail) with SMTP id cC_KCgCH_r1VB9NkdqOEDA--.19394S4; Wed, 09 Aug 2023 11:26:14 +0800 (CST) From: Lin Ma To: corbet@lwn.net, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, rdunlap@infradead.org, void@manifault.com, jani.nikula@intel.com Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Lin Ma Subject: [PATCH v2] docs: staging: add netlink attrs best practices Date: Wed, 9 Aug 2023 11:25:52 +0800 Message-Id: <20230809032552.765663-1-linma@zju.edu.cn> X-Mailer: git-send-email 2.17.1 X-CM-TRANSID: cC_KCgCH_r1VB9NkdqOEDA--.19394S4 X-Coremail-Antispam: 1UD129KBjvAXoWfZry8WrWfGF1xXry7Kry8Zrb_yoW5uFW5Ko WS9w45Cw48tr13CFWYyr1kAF9xAayDWFyxJr4a9398W3WDXanIk345Cw4kWa4fAF45Kanx Ja48Jw18ZFn0qF95n29KB7ZKAUJUUUUU529EdanIXcx71UUUUU7v73VFW2AGmfu7bjvjm3 AaLaJ3UjIYCTnIWjp_UUUY77AC8VAFwI0_Gr0_Xr1l1xkIjI8I6I8E6xAIw20EY4v20xva j40_Wr0E3s1l1IIY67AEw4v_Jr0_Jr4l8cAvFVAK0II2c7xJM28CjxkF64kEwVA0rcxSw2 x7M28EF7xvwVC0I7IYx2IY67AKxVWDJVCq3wA2z4x0Y4vE2Ix0cI8IcVCY1x0267AKxVW8 Jr0_Cr1UM28EF7xvwVC2z280aVAFwI0_GcCE3s1l84ACjcxK6I8E87Iv6xkF7I0E14v26r xl6s0DM2AIxVAIcxkEcVAq07x20xvEncxIr21l5I8CrVACY4xI64kE6c02F40Ex7xfMcIj 6xIIjxv20xvE14v26r1j6r18McIj6I8E87Iv67AKxVWUJVW8JwAm72CE4IkC6x0Yz7v_Jr 0_Gr1lF7xvr2IYc2Ij64vIr41lF7I21c0EjII2zVCS5cI20VAGYxC7M4IIrI8v6xkF7I0E 8cxan2IY04v7MxAIw28IcxkI7VAKI48JMxC20s026xCaFVCjc4AY6r1j6r4UMI8I3I0E5I 8CrVAFwI0_Jr0_Jr4lx2IqxVCjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AKxVWUtVW8 ZwCIc40Y0x0EwIxGrwCI42IY6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI8IcVCY1x 0267AKxVW8JVWxJwCI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAFwI0_ Jr0_Gr1lIxAIcVC2z280aVCY1x0267AKxVW8JVW8JrUvcSsGvfC2KfnxnUUI43ZEXa7VUb XdbUUUUUU== X-CM-SenderInfo: qtrwiiyqvtljo62m3hxhgxhubq/ Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Provide some suggestions that who deal with Netlink code could follow (of course using the word "best-practices" may sound somewhat exaggerate). According to my recent practices, the parsing of the Netlink attributes lacks documents for kernel developers. Since recently the relevant docs for Netlink user space get replenished, I guess is a good chance for kernel space part to catch with. First time to write a document and any reviews are appreciated. Signed-off-by: Lin Ma --- v1 -> v2: fix some typos in FOO example, add extra section "About General Netlink Case" to avoid any confusion for new code users. Documentation/staging/index.rst | 1 + .../staging/netlink-attrs-best-practices.rst | 544 ++++++++++++++++++ MAINTAINERS | 1 + 3 files changed, 546 insertions(+) create mode 100644 Documentation/staging/netlink-attrs-best-practices.rst diff --git a/Documentation/staging/index.rst b/Documentation/staging/index.= rst index ded8254bc0d7..683c2b37e82c 100644 --- a/Documentation/staging/index.rst +++ b/Documentation/staging/index.rst @@ -8,6 +8,7 @@ Unsorted Documentation =20 crc32 lzo + netlink-attrs-best-practices remoteproc rpmsg speculation diff --git a/Documentation/staging/netlink-attrs-best-practices.rst b/Docum= entation/staging/netlink-attrs-best-practices.rst new file mode 100644 index 000000000000..7dac562bee47 --- /dev/null +++ b/Documentation/staging/netlink-attrs-best-practices.rst @@ -0,0 +1,544 @@ +.. SPDX-License-Identifier: BSD-3-Clause + +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D +Netlink Attributes Best Practices +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D + +Introduction +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +This document serves as a guide to the best practices, or cautions, for pa= rsing user-space-provided Netlink attributes in kernel space. The intended = audience is those who want to leverage the powerful Netlink interface (main= ly for classic or legacy Netlink and the general Netlink users basically do= n't worry about these, see penultimate section) in their code. Additionally= , for those who are curious about the parsing of Netlink attributes but may= feel apprehensive about delving into the code itself, this document can se= rve as an excellent starting point. + +However, if you are concerned about how to prepare Netlink messages from a= user space socket instead of writing kernel code, it is recommended to rea= d the `Netlink Handbook `_ first. + +Background +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +Data Structures +--------------- + +So what is a Netlink attribute? In simple terms, **the Netlink attribute i= s the structural payload carried by the Netlink message in TLV (Type-Length= -Value) format** (At least it is suggested to do so). One can straightly re= ad the comment and the code in ``include/net/netlink.h`` (most of the below= content is derived from there). The following graph demonstrates the struc= ture for the majority of messages. + +.. code-block:: + + +-----------------+------------+-------------------------- + | Netlink Msg Hdr | Family Hdr | Netlink Attributes ... + +-----------------+------------+-------------------------- + ^ ^ + +The ``^`` part will be padded to align to ``NLMSG_ALIGNTO`` (4 bytes for t= he Linux kernel). + +The term **majority** is used here because the structure is dependent on t= he specific Netlink family you are dealing with. For example, the general N= etlink family (NETLINK_GENERIC) puts ``struct genlmsghdr`` in the Family Hd= r location and is strictly followed by specified Netlink attributes TLV. Di= fferently, the connector Netlink family (NETLINK_CONNECTOR) does not use Ne= tlink attributes for transiting the payload, but straight places naked data= structure after its family header ``struct cn_msg``. In general, when work= ing with Netlink-powered code, most developers opt for Netlink attributes d= ue to their convenience and ease of maintenance. This means is definitely o= kay to overlook the corner cases which may eventually be incorporated into = Netlink attributes in the future. + +The Netlink attribute in the Linux kernel conforms to the following struct= ure. + +.. code-block:: c + + // >------- nla header --------< + // +-------------+-------------+----------- - - - ----------+ + // | Attr Len | Attr Type | Attr Data | + // +-------------+-------------+----------- - - - ----------+ + // >-- 2 bytes --|-- 2 bytes --|------ Attr Len bytes ------< + + struct nlattr { + __u16 nla_len; + __u16 nla_type; + }; + +The 2 bytes attr len (\ ``nla_len``\ ) indicates the entire attribute leng= th (includes the nla header) and the other 2 bytes attr type (\ ``nla_type`= `\ ) is used by the kernel code to identify the expected attribute. To proc= ess these attributes, the kernel code needs to locate the specific attribut= e and extract the payload from it, a process known as attribute parsing. Th= is procedure can be tedious and error-prone when done manually, so the inte= rface provides parsers to simplify the process. + +*It is worth mentioning that if you choose to use general Netlink without = a nested data structure, you don't even need to call any parsers as the int= erface already does this and your task will be simplified to handling the p= arsed result.* + +Parsers +------- + +There are several parsers available, each designed to handle a specific ty= pe of object being parsed. If you have a pointer to a Netlink message, spec= ifically a (\ ``struct nlmsghdr *``\ ), it's likely that you'll want to cal= l the ``nlmsg_parse`` function. The prototype for this function is as follo= ws: + +.. code-block:: c + + /** + * nlmsg_parse - parse attributes of a netlink message + * @nlh: netlink message header + * @hdrlen: length of family specific header + * @tb: destination array with maxtype+1 elements + * @maxtype: maximum attribute type to be expected + * @policy: validation policy + * @extack: extended ACK report struct + * + * See nla_parse() + */ + static inline int nlmsg_parse(const struct nlmsghdr *nlh, int hdrlen, + struct nlattr *tb[], int maxtype, + const struct nla_policy *policy, + struct netlink_ext_ack *extack); + +Else if you have a pointer to Netlink attribute (\ ``struct nlattr *``\ ) = already, the ``nla_parse``\ , or ``nla_parse_nested`` could be your choice= s. + +.. code-block:: c + + /** + * nla_parse - Parse a stream of attributes into a tb buffer + * @tb: destination array with maxtype+1 elements + * @maxtype: maximum attribute type to be expected + * @head: head of attribute stream + * @len: length of attribute stream + * @policy: validation policy + * @extack: extended ACK pointer + * + * Parses a stream of attributes and stores a pointer to each attribute= in + * the tb array accessible via the attribute type. Attributes with a ty= pe + * exceeding maxtype will be rejected, policy must be specified, attrib= utes + * will be validated in the strictest way possible. + * + * Returns 0 on success or a negative error code. + */ + static inline int nla_parse(struct nlattr **tb, int maxtype, + const struct nlattr *head, int len, + const struct nla_policy *policy, + struct netlink_ext_ack *extack); + /** + * nla_parse_nested - parse nested attributes + * @tb: destination array with maxtype+1 elements + * @maxtype: maximum attribute type to be expected + * @nla: attribute containing the nested attributes + * @policy: validation policy + * @extack: extended ACK report struct + * + * See nla_parse() + */ + static inline int nla_parse_nested(struct nlattr *tb[], int maxtype, + const struct nlattr *nla, + const struct nla_policy *policy, + struct netlink_ext_ack *extack); + +Upon closer inspection, one will notice that the parameters for the variou= s parsers bear a striking resemblance to one another. In fact, they share a= commonality that goes beyond mere coincidence, as they all ultimately call= upon the same internal parsing helper function, namely ``__nla_parse``. + +.. code-block:: c + + int __nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *h= ead, + int len, const struct nla_policy *policy, unsigned int validate, + struct netlink_ext_ack *extack); + +The idea is straightforward since we know that adding an offset to either = the Netlink message header or the nested attribute will yield the TLV forma= t of the attributes. On this basis, we will learn how to utilize those pars= ers. + +The parser functions mentioned above require two inputs from the user: a d= estination array, ``tb``, and a maximum attribute type, ``maxtype``. Rest a= ssured that the parsers will take care of clearing the buffer by calling me= mset, so you don't need to worry about any dirty data. After the parsing, = parsers will store the parsed pointer in the provided array, allowing you t= o easily access the parsed data later on. For clarity, you can refer to th= e example below to see how the parsed result is organized. + +.. code-block:: c + + enum { + EXAMPLEA_1, + EXAMPLEA_2, + EXAMPLEA_3, + EXAMPLEA_4, + __EXAMPLEA_MAX + + #define EXAMPLEA_MAX (__EXAMPLEA_MAX - 1) + }; + + /* + * attributes being parsed and pointers: + * + * +---+------------+- - -+---+------------+- - -+---+------------+= - - -+ + * | X | EXAMPLEA_1 | ... | Y | EXAMPLEA_4 | ... | Z | EXAMPLEA_2 |= ... | + * +---+------------+- - -+---+------------+- - -+---+------------+= - - -+ + * ^ ^ ^ + * | | | + * ptr1 | | + * ptr2 -----------------------------------------+ + * ptr3 =3D NULL | + * ptr4 ------------------+ + */ + + struct nlattr *tb[EXAMPLEA_MAX+1]; + nla_parse(tb, EXAMPLEA_MAX, ....); + // After the parser returns, the tb contains the data shown below, + // with each pointer holding the value indicated in the above diagram. + // tb =3D=3D { + // ptr1, // EXAMPLEA_1 + // ptr2, // EXAMPLEA_2 + // NULL, // EXAMPLEA_3 + // ptr4, // EXAMPLEA_4 + // }; + +The Linux kernel's attribute parsing mechanism is notably flexible, enabli= ng attributes to be presented in any order, regardless of their specific ty= pe (e.g., ``EXAMPLE_4`` can precede ``EXAMPLE_2``). This flexibility is not= always the case in other Netlink implementations, such as FreeBSD, where a= ttributes may be required to follow a specific order. + +With the parsing process complete, the kernel code can readily access the = parsed attribute using the attribute pointer. To illustrate, let's say we a= nticipate the ``EXAMPLEA_2`` attribute to hold a ``u16`` value. In that cas= e, we can derive it in the following manner: + +.. code-block:: c + + u16 val; + if (tb[EXAMPLEA_2]) // check if user provides attribute EXAMPLEA_2 + val =3D nla_get_u16(tb[EXAMPLEA_2]); // dereference it + +There are multiple helpers available to help us to derive the attribute, s= uch as ``nla_get_u8/16/32/64``\ , ``nla_get_s8/16/32/64``\ , ``nla_get_flag= ``\ , ``nla_get_msecs``. + +Looks pretty good, right? However, the code still has a potential flaw, as= it naively assumes that the user has indeed placed a ``u16`` value in the = payload. We'll discuss this issue further in the #2 best practice. + +FOO example +----------- + +In this part we will make a horizontal comparison to help readers better u= nderstand how to write a Netlink code to parse the user provided data. o be= gin with, let's take a look at how this is accomplished in ``ioctl``. + +.. code-block:: c + + struct foo_d1 { + u32 a; + u32 b; + }; + + struct foo_d2 { + u32 c; + u32 d; + u8 haschild; + struct foo_d1 child; // optional + } + + static long foo_ioctl(struct file *flip, unsigned int cmd, unsigned lon= g arg) + { + u32 s =3D offsetof(struct foo_d2, child); + struct foo_d2 *d2 =3D kmalloc(sizeof(struct foo_d2), GFP_KERNEL); + if (!d2) return -ENOMEM; + + copy_from_user(d2, arg, s); + // access data like d2->c, d2->d + + if (d2->haschild) { + copy_from_user(&d2->child, arg + s, sizeof(struct foo_d1)); + // access data like d2->child.a, d2->child.b + } + // ... + } + +The ``ioctl`` code above reveals that two calls to ``copy_from_user`` are = made, facilitating the transfer of data from user space to kernel space. Af= ter these calls, the code can manipulate values using pointers. + +n contrast, the second snippet showcases a similar solution when utilizing= the Netlink interface. While there are multiple ways to use the attributes= to achieve the same outcome, the snippet below illustrates just one possib= le approach. + +.. code-block:: c + + + enum { + FOOCHILD_A, + FOOCHILD_B, + __FOOCHILD_MAX, + + #define FOOCHILD_MAX (__FOOCHILD_MAX - 1) + }; + + enum { + FOOA_C, + FOOA_D, + FOOA_CHILD, + __FOOA_MAX, + + #define FOOA_MAX (__FOOA_MAX - 1) + }; + + // just an example, different consumers have different handler prototype + static int foo_handler(struct nlmsghdr *nlh, struct netlink_ext_ack *ex= tack) + { + struct nlattr *attrs[FOOA_MAX+1]; + int err; + // ... + err =3D nlmsg_parse(nlh, FOOA_MAX, attrs, ..., extack); + // ... + if (!attrs[FOOA_C] || !attrs[FOOA_D]) + return -EINVAL; + // access data via + // nla_get_u32(attrs[FOOA_C]) and nla_get_u32(attrs[FOOA_D]) + + // access child as nested + if (attrs[FOOA_CHILD]) + err =3D foo_handler_internal(attrs[FOOA_ARRAY], extack); + } + + static int foo_handler_internal(struct nlattr *nla, struct netlink_ext_= ack *extack) + { + struct nlattr *attrs[FOOCHILD_MAX+1]; + int err; + // ... + err =3D nla_parse_nested(attrs, FOOCHILD_MAX, nla, ..., extack); + // access data via + // nla_get_u32(attrs[FOOCHILD_A]), nla_get_u32(attrs[FOOCHILD_B]) + } + +By contrast, the Netlink version provides several benefits, including impr= oved readability, thanks to the use of enums that clearly identify each val= ue, and simplified writing, as the interface takes care of transferring dat= a to kernel space, allowing programmers to focus on other parts of the code= without worrying about bugs like double fetching. + +Now that we've covered the background information and basic data structure= , it's time to dive deeper into the best practices for handling Netlink att= ributes. If you're interested in learning more about the underlying code, w= e recommend checking out the ``lib/nlattr.c`` file for additional details. = With that said, let's move on to the tips and tricks for working with Netli= nk attributes. + +Best Practices +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +#1 Use provided helpers to parse and access nlattr +-------------------------------------------------- + +As mentioned above, the Netlink attributes parsing procedure operates on p= ointers, which is rather error-prone. To avoid introducing bugs, as well as= enhance code maintainability, it's recommended to utilize the provided par= sers and helpers when working with Netlink attributes. + +While it's generally recommended to use existing parsers to parse Netlink = attributes, there may be (old) cases where kernel code needs to manually pa= rse the attributes. We'll discuss this further later on. Regardless, it's s= uggested to use helpers to perform the access action for better code mainta= inability and reliability. + +For example, one should never add the offset to the attribute pointer hims= elf. + +.. code-block:: c + + struct nlattr *nla =3D ...; + u8 *p; + + p =3D (char*)nla + sizeof(struct nlattr); // Correct but not suggested + p =3D nla_data(nla); // Correct as well as concise + +Several helpers are particularly useful but often overlooked, including ``= nla_memcpy``\ , ``nla_memdup``\ , ``nla_memcmp``\ , and ``nla_strscpy``\ , = ``nla_strdup``\ , ``nla_strcmp``. These helpers can greatly simplify the pr= ocess of working with Netlink attributes and reduce the risk of errors. + +.. code-block:: c + + struct nlattr *nla =3D ...; + u8 buffer[XXX]; + + memcpy(buffer, nla_data(nla), XXX); // Wordy and could be buggy + nla_memcpy(buffer, nla, XXX); // Concise and also safe + +When you need to access a Netlink attribute, it's recommended to check if = there's already a helper function available in the ``include/net/netlink.h`= `\ header file. This will help you avoid introducing complex and error-pro= ne pointer arithmetic into your code. + + +#2 Never access nlattr without checking +--------------------------------------- + +Let's take another look at the example code for utilizing parsers. + +.. code-block:: c + + u16 val; + if (tb[EXAMPLEA_2]) + val =3D nla_get_u16(tb[EXAMPLEA_2]); + +Recall that we noted earlier that this access code has a bug since it naiv= ely trusts the input provided and assumes that a ``u16`` value is present. = Now, imagine a scenario where a malicious user deliberately provides a malf= ormed ``EXAMPLEA_2``, as shown below. + +.. code-block:: + + nla_len nla_type + +-------+------------+- - - - - - - - - + + | 0 | EXAMPLEA_2 | Heap Dirty Data | + +-------+------------+- - - - - - - - - + + ^ + attributes end location + +In this scenario, the attribute ``EXAMPLEA_2`` is positioned as the final = attribute, situated adjacent to the heap dirty data (since the nlmsg is all= ocated with the ``GFP_KERNEL`` flag without ``GFP_ZERO``\ ). Furthermore, a= lthough the nla_type is ``EXAMPLEA_2`` as exptected, the nla_len is ``0``, = rather than the expected ``sizeof(u16)``, in the malformed packet. + +The access code erroneously believes the parsed attribute has a valid leng= th, leading to out-of-attribute access. This has serious consequences, as i= t causes the heap of dirty data to be loaded into the variable ``val``\ , w= hich can be stored in a global state and accessed from user space. This lea= kage of heap information can be exploited by a skilled attacker using Heap = Feng Shui, then gain unauthorized access to sensitive data, potentially all= owing them to bypass protections like KASLR or SLAB_HARDEN. + +To prevent such a problem from occurring, it's essential to verify the att= ribute before attempting to access it. For instance, we can modify the exam= ple code as follows: + +.. code-block:: c + + u16 val; + if (tb[EXAMPLEA_2] \ + && nla_len(tb[EXAMPLEA_2]) >=3D sizeof(u16)) // length check + val =3D nla_get_u16(tb[EXAMPLEA_2]); + +The similar case is the example using ``memcpy`` to access the attribute. + +.. code-block:: c + + u8 buffer[XXX]; + memcpy(buffer, nla_data(nla), XXX); + +The code mistakenly assumes that the ``nla`` has a length of ``XXX``\ , le= ading to out-of-attribute access that potentially copies dirty data to the = ``buffer``\ . However, ``nla_memcpy`` is immune to such bugs because it int= ernally calls ``nla_len`` to ensure that only data within the boundary is c= opied. This also underscores the importance of #1 best practice. + +While the length check, which ensures the attribute has enough data to be = accessed by `nla_get_u16`, is a good starting point, it's not the most effi= cient approach to perform such checks extensively throughout the code. More= over, there are other important factors to consider beyond data length, suc= h as value ranges, when determining the validity of an attribute. In realit= y, data length is just one of the basic aspects we need to examine. + +To achieve more convenient and flexible validation, a very important data = structure named ``nla_policy`` is employed. + +.. code-block:: c + + struct nla_policy { + u8 type; + u8 validation_type; + u16 len; + union { + /** + * ... field for advanced validation ... + */ + }; + }; + +There are two ways to employ ``nla_policy``: pass the policy to the parser= , which will validate the parsing process (see the ``policy`` parameter in = those parsers), or actively call validation helpers like ``nlmsg_validate``= and ``nla_validate`` to check the attributes. The first option is generall= y preferred because it ensures that the pointer in the parsed destination a= rray points to a valid attribute that can be safely accessed, eliminating t= he need for an additional check. Here's an example: + +.. code-block:: c + + struct nla_policy example_policy[EXAMPLEA_MAX + 1] =3D { + [EXAMPLEA_2] =3D { .type =3D NLA_U8 }, + }; + // ... + struct nlattr *tb[EXAMPLEA_MAX+1]; + u16 value; + nla_parse(tb, EXAMPLEA_MAX, nla, ..., example_policy, extack); + + u16 val; + if (tb[EXAMPLEA_2]) // if not NULL, already pass the policy length check + val =3D nla_get_u16(tb[EXAMPLEA_2]); // safe + +In addition to ``NLA_U8``\ , there are several other defined types, such a= s ``NLA_BINARY`` and ``NLA_STRING``, that can be used to prepare a nla_poli= cy. However, two attributes deserve special attention: ``NLA_NESTED`` and `= `NLA_NESTED_ARRAY``. These attributes are used to validate complex structur= es that contain nested elements, and they are particularly useful when deal= ing with hierarchical data structures. For instance, we can define a ``nla_= policy`` for the Foo example above using these attributes. + +.. code-block:: c + + struct nla_policy foo_policy_nested[FOOCHILD_MAX + 1] =3D { + [FOOCHILD_A] =3D { .type =3D NLA_U32 }, + [FOOCHILD_B] =3D { .type =3D NLA_U32 }, + }; + + struct nla_policy foo_policy[EXAMPLEA_MAX + 1] =3D { + [FOOA_C] =3D { .type =3D NLA_U32 }, + [FOOA_D] =3D { .type =3D NLA_U32 }, + [FOOA_CHILD] =3D NLA_POLICY_NESTED(foo_policy_nested), + }; + +``nla_policy`` by no means make the validation clean and graceful hence sh= ould be employed first than manual checks. The general Netlink interface al= lows the user to specify the policy for each handler and finish the validat= ion before calling it, we will talk more about this next section. + +While using ``nla_policy`` is generally straightforward and recommended, t= here are some important considerations to keep in mind. + +First and foremost,, **Don't forget to complement the nla_policy when a ne= w attribute type is introduced**. There are cases when developers, no matte= r for classic Netlink or general Netlink interfaces, make such mistakes. If= the parsing does not follow the strict mode (which will be discussed in #3= ), such a mistake will result in the out-of-attribute access bug. + +Second, **Perform manual checks for manual parsing**. Examine the kernel c= ode, readers can find cases that prepare ``nla_policy`` in a unconventional= manner like below. + +.. code-block:: c + + // drivers/net/bonding/bond_netlink.c + static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] =3D { + // ... + [IFLA_BOND_ARP_IP_TARGET] =3D { .type =3D NLA_NESTED }, + }; + +The attribute ``IFLA_BOND_ARP_IP_TARGET`` is defined with the ``NLA_NESTED= `` type, but without a nested ``nla_policy``. While it may seem logical to = one to allow nested validation for this attribute, the answer is, unfortuna= tely, no. The code employs an undocumented and sophisticated approach to or= ganizing attributes, where the array index is embedded within the attribute= type. Currently, there is no validation mechanism available to handle such= a case. + +Besides, there may be other cases where the kernel code also deviates from= the conventional approach of using parsers and instead manually iterates t= hrough the attributes TLV, performing manual parsing. + +To validate cases where manual parsing is used, manual length checks based= on ``nla_len`` are required, as demonstrated in the parsing code for ``IFL= A_BOND_ARP_IP_TARGET`` below. + +.. code-block:: c + + if (data[IFLA_BOND_ARP_IP_TARGET]) { + // ... + nla_for_each_nested(attr, data[IFLA_BOND_ARP_IP_TARGET], rem) { + __be32 target; + + // manual length check here + if (nla_len(attr) < sizeof(target)) + return -EINVAL; + + target =3D nla_get_be32(attr); + // ... + +``NLA_POLICY_NESTED`` is just one of many helpful tools at your disposal. = If you're looking to validate the value range of an attribute, consider usi= ng ``NLA_POLICY_RANGE``\ , ``NLA_POLICY_MIN``\ , and ``NLA_POLICY_MAX``\ . = And if you need to perform a strict length check, ``NLA_POLICY_EXACT_LEN`` = is your best bet. Simply assigning a value to len in nla_policy will ensure= that the attribute length is greater than or equal to your expected value,= without strictly equaling it. + +#3 Embrace and take advantage of the strictness +----------------------------------------------- + +In the previous section, we talked about **strict mode**, which is related= to the last best practice we'll discuss. While reading through the helpers= declared in ``include/net/netlink.h``\ , readers may have noticed some of = them have the ``_deprecated`` suffix, such as ``nla_parse_deprecated`` and = ``nlmsg_parse_deprecated``. These deprecated functions are still widely use= d in the kernel, with over 300 call sites. This raises some questions: why = are they marked as deprecated? What's the difference between them and the n= on-deprecated versions? And how do we choose between them? + +These deprecated functions in question were introduced in commit **8cb0817= 46c03 ("netlink: make validation more configurable for future strictness")*= *, which aimed to enhance the configurability of parsing strictness. This c= ommit split the parsing strictness into several options, as detailed in the= commit message. + +* TRAILING - check that there's no trailing data after parsing attribu= tes (in message or nested) +* MAXTYPE - reject attrs greater than max known type +* UNSPEC - reject attributes with NLA_UNSPEC policy entries +* STRICT_ATTRS - strictly validate attribute size + +and one more option follows up later + +* NESTED - strictly validate ``NLA_F_NESTED`` + +After the aforementioned commit, the code now features three distinct leve= ls of strictness: Liberal, Deprecated Strict, and Strict. By examining the = code, it becomes readily apparent which options are employed by each level. + +.. code-block:: c + + enum netlink_validation { + NL_VALIDATE_LIBERAL =3D 0, + NL_VALIDATE_TRAILING =3D BIT(0), + NL_VALIDATE_MAXTYPE =3D BIT(1), + NL_VALIDATE_UNSPEC =3D BIT(2), + NL_VALIDATE_STRICT_ATTRS =3D BIT(3), + NL_VALIDATE_NESTED =3D BIT(4), + }; + + #define NL_VALIDATE_DEPRECATED_STRICT (NL_VALIDATE_TRAILING |\ + NL_VALIDATE_MAXTYPE) + #define NL_VALIDATE_STRICT (NL_VALIDATE_TRAILING |\ + NL_VALIDATE_MAXTYPE |\ + NL_VALIDATE_UNSPEC |\ + NL_VALIDATE_STRICT_ATTRS |\ + NL_VALIDATE_NESTED) + +The ``NL_VALIDATE_UNSPEC`` is an interesting one that deserves a note. In = the previous part, a buggy case caused by forgetting to complement the poli= cy is mentioned. Specifically, such a case only occurs when using the depre= cated level. If choosing the latest strict level, which enables the ``NL_VA= LIDATE_UNSPEC`` option, such a case can be prevented as the newly added att= ribute type will be directly rejected due to the absence of the policy. Thi= s is a compelling reason to use the modern strict level instead of the olde= r ones. + +While the benefits of using ``NL_VALIDATE_STRICT`` are clear, simply repla= cing all deprecated parsers with this new option could result in compatibil= ity issues due to the fact that older user space code may not follow a stri= ct implementation. To address this concern, Johannes Berg, the author of th= e aforementioned commit, proposed a subsequent commit **56738f460841 ("netl= ink: add strict parsing for future attributes")**, which introduces a new f= ield, strict_start_type, in the struct nla_policy structure. This new field= allows for the definition of a "boundary type" for legacy code that utiliz= es deprecated parsers, thereby ensuring compatibility with older code while= still providing the benefits of strict parsing. + + +About General Netlink Case +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D + +The aforementioned content primarily presents best practices based on the = author's experience with classic Netlink. However, these practices may not = be relevant or could potentially cause confusion when working with new code= that employs general Netlink. In the following section, we will explore ho= w users of general Netlink can address these concerns and identify addition= al considerations that need to be taken into account. + +Let's revisit the FOO example, where the general Netlink alternative would= look something like this. + +.. code-block:: c + + static const struct genl_split_ops foo_nl_ops[] =3D { + { + .cmd =3D FOO_CMD_TEST, + .doit =3D foo_nl_test_doit, + .policy =3D foo_policy, + .maxattr =3D EXAMPLEA_MAX, + .flags =3D GENL_ADMIN_PERM, + // .validate =3D GENL_DONT_VALIDATE_STRICT + }, + }; + + int foo_nl_test_doit(struct sk_buff *skb, struct genl_info *info) + { + struct nlattr *tb_child[FOOCHILD_MAX + 1]; + + /* can access parsed result in info->attrs[] */ + // nla_get_u32(info->attrs[FOOA_C]) + if (info->attrs[FOOA_CHILD]) { + if (nla_parse_nested(tb_child, FOOCHILD_MAX, + info->attrs[FOOA_CHILD], foo_policy_neste= d, NULL)) + return -EINVAL + /* can access nested parsed result in tb_child[] */ + // nla_get_u32(tb_child[FOOCHILD_A]) + } + } + + /* omit and struct genl_family and call to genl_register_family */ + +In contrast to the classic implementation, which primarily involves regist= ering a callback function, general Netlink utilizes the ``genl_split_ops`` = structure (or ``genl_ops``, ``genl_small_ops`` depending on the situation) = to provide additional details such as privileges, concurrency, and more. In= the example mentioned above, when the ``policy`` and ``maxattr`` are known= , the Netlink core will perform validation and parsing tasks before invokin= g the ``doit`` callback. + +As a result, the general Netlink user inadvertently follows the second bes= t practice once they specify the ``policy`` field. Furthermore, in the case= of modern families where the specification is generated (see `Netlink spec= C code generation `_ ), the problem of forgetting to complement the ``nla_policy`` sho= uld never occur. This feature is incredibly beneficial as it enhances maint= ainability. It is highly recommended to incorporate it into your Netlink co= de. + +However, the general Netlink user should still consider the #1 and #3 best= practices as areas of concern. + +About the first best practice, regardless of the interface you are current= ly working with, it is essential to handle Netlink attributes and make use = of the available helpers. definitely don't forget the helper ``nla_memcpy``. + +Regarding the third best practice, general Netlink employs a modern strict= ness approach by default for parsing, utilizing the user-provided policy an= d maxattr, which is perfect. However, for the sake of flexibility (and comp= atibility), the interface also allows users to specify ``GENL_DONT_VALIDATE= _STRICT`` if they prefer a more liberal strictness. Additionally, there are= additional validate flags available, see ``genl_validate_flags`` enum. + +.. code-block:: c + + enum genl_validate_flags { + GENL_DONT_VALIDATE_STRICT =3D BIT(0), + GENL_DONT_VALIDATE_DUMP =3D BIT(1), + GENL_DONT_VALIDATE_DUMP_STRICT =3D BIT(2), + }; + +To be honest, the developer should avoid those ``DONT`` flags as possible.= However, the ``GENL_DONT_VALIDATE_STRICT`` flag, which enables liberal str= ictness, is used over 400 times in the Linux kernel code. This observation = suggests that a significant portion of the codebase has not fully adopted m= odern strictness principles yet. + +Additionally, also illustrated by the FOO example, the user has to do loca= l parsing when processing nested attributes. In such scenarios, please + +* leverage the parsers instead of manual parsing. +* leverage the parsers which are not marked as deprecated to adhere to mod= ern strictness. + +Summary +=3D=3D=3D=3D=3D=3D=3D + +This document provides an overview of the data structure and parsers for N= etlink attributes, along with three best practices for parsing these attrib= utes. The parsing discussed in this document only applies to the "RX channe= l" (most ``doit`` functions), where the kernel needs to parse user space-pr= ovided Netlink messages. The opposite channel, where the kernel prepares Ne= tlink messages for user space (most ``dumpit`` functions), is not covered i= n this document as it is considered less prone to implementation errors. + +The first best practice is "use provided helper to parse and access nlattr= ", which can make the parsing easier and increase the code maintainability.= The second one is "never access nlattr without checking", otherwise an out= -of-attribute access bug could occur. The last one is "embrace and take adv= antage of the strictness", which has been a topic of discussion since 2019 = but has yet to be fully adopted. + +An extra section is used to discuss the general Netlink interface case. Fo= rtunately, this interface has evolved rapidly, becoming safer and safer. Th= e latest general Netlink allows for the generation of specifications and po= licy automatically from YAML, with the generated policy defaulting to moder= n strictness. However, users still have to concern about the #1 and #3 prac= tices when dealing with the attributes. + +It's important to note that correctly parsing Netlink attributes is only t= he first step but definitely not the last step to keep your code away from = the bugs. diff --git a/MAINTAINERS b/MAINTAINERS index 0f966f05fb0d..129f5c3abdd0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14729,6 +14729,7 @@ F: Documentation/core-api/netlink.rst F: Documentation/netlink/ F: Documentation/networking/ F: Documentation/process/maintainer-netdev.rst +F: Documentation/staging/netlink-attrs-best-practices.rst F: Documentation/userspace-api/netlink/ F: include/linux/in.h F: include/linux/net.h --=20 2.17.1