Commit ff62b792 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'ynl-avoid-leaks-in-attr-override-and-spec-fixes-for-c' 

Jakub Kicinski says:

====================
ynl: avoid leaks in attr override and spec fixes for C

The C rt-link work revealed more problems in existing codegen
and classic netlink specs.

Patches 1 - 4 fix issues with the codegen. Patches 1 and 2 are
pre-requisites for patch 3. Patch 3 fixes leaking memory if user
tries to override already set attr. Patch 4 validates attrs in case
kernel sends something we don't expect.

Remaining patches fix and align the specs. Patch 5 changes nesting,
the rest are naming adjustments.
====================

Link: https://patch.msgid.link/20250414211851.602096-1-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 00ffb372 e31f86ee

Documentation/netlink/specs/rt_link.yaml

+13 −7
Original line number Diff line number Diff line
@@ -1113,11 +1113,10 @@ attribute-sets: 
      -

        name: prop-list

        type: nest

        nested-attributes: link-attrs

        nested-attributes: prop-list-link-attrs

      -

        name: alt-ifname

        type: string

        multi-attr: true

      -

        name: perm-address

        type: binary
@@ -1163,6 +1162,13 @@ attribute-sets: 
      -

        name: netns-immutable

        type: u8

  -

    name: prop-list-link-attrs

    subset-of: link-attrs

    attributes:

      -

        name: alt-ifname

        multi-attr: true

  -

    name: af-spec-attrs

    attributes:
@@ -1585,7 +1591,7 @@ attribute-sets: 
        name: nf-call-iptables

        type: u8

      -

        name: nf-call-ip6-tables

        name: nf-call-ip6tables

        type: u8

      -

        name: nf-call-arptables
@@ -2077,7 +2083,7 @@ attribute-sets: 
        name: id

        type: u16

      -

        name: flag

        name: flags

        type: binary

        struct: ifla-vlan-flags

      -
@@ -2165,7 +2171,7 @@ attribute-sets: 
        type: binary

        struct: ifla-cacheinfo

      -

        name: icmp6-stats

        name: icmp6stats

        type: binary

        struct: ifla-icmp6-stats

      -
@@ -2179,9 +2185,10 @@ attribute-sets: 
        type: u32

  -

    name: mctp-attrs

    name-prefix: ifla-mctp-

    attributes:

      -

        name: mctp-net

        name: net

        type: u32

      -

        name: phys-binding
@@ -2453,7 +2460,6 @@ operations: 
            - min-mtu

            - max-mtu

            - prop-list

            - alt-ifname

            - perm-address

            - proto-down-reason

            - parent-dev-name

Documentation/netlink/specs/rt_neigh.yaml

+7 −7
Original line number Diff line number Diff line
@@ -13,25 +13,25 @@ definitions: 
    type: struct

    members:

      -

        name: family

        name: ndm-family

        type: u8

      -

        name: pad

        name: ndm-pad

        type: pad

        len: 3

      -

        name: ifindex

        name: ndm-ifindex

        type: s32

      -

        name: state

        name: ndm-state

        type: u16

        enum: nud-state

      -

        name: flags

        name: ndm-flags

        type: u8

        enum: ntf-flags

      -

        name: type

        name: ndm-type

        type: u8

        enum: rtm-type

  -
@@ -189,7 +189,7 @@ attribute-sets: 
        type: binary

        display-hint: ipv4

      -

        name: lladr

        name: lladdr

        type: binary

        display-hint: mac

      -

tools/net/ynl/pyynl/ynl_gen_c.py

+72 −24
Original line number Diff line number Diff line
@@ -162,9 +162,15 @@ class Type(SpecAttr): 
    def free_needs_iter(self):

        return False



    def free(self, ri, var, ref):

    def _free_lines(self, ri, var, ref):

        if self.is_multi_val() or self.presence_type() == 'len':

            ri.cw.p(f'free({var}->{ref}{self.c_name});')

            return [f'free({var}->{ref}{self.c_name});']

        return []



    def free(self, ri, var, ref):

        lines = self._free_lines(ri, var, ref)

        for line in lines:

            ri.cw.p(line)



    def arg_member(self, ri):

        member = self._complex_member_type(ri)
@@ -263,6 +269,10 @@ class Type(SpecAttr): 
        var = "req"

        member = f"{var}->{'.'.join(ref)}"



        local_vars = []

        if self.free_needs_iter():

            local_vars += ['unsigned int i;']



        code = []

        presence = ''

        for i in range(0, len(ref)):
@@ -272,6 +282,10 @@ class Type(SpecAttr): 
            if i == len(ref) - 1 and self.presence_type() != 'bit':

                continue

            code.append(presence + ' = 1;')

        ref_path = '.'.join(ref[:-1])

        if ref_path:

            ref_path += '.'

        code += self._free_lines(ri, var, ref_path)

        code += self._setter_lines(ri, member, presence)



        func_name = f"{op_prefix(ri, direction, deref=deref)}_set_{'_'.join(ref)}"
@@ -279,7 +293,8 @@ class Type(SpecAttr): 
        alloc = bool([x for x in code if 'alloc(' in x])

        if free and not alloc:

            func_name = '__' + func_name

        ri.cw.write_func('static inline void', func_name, body=code,

        ri.cw.write_func('static inline void', func_name, local_vars=local_vars,

                         body=code,

                         args=[f'{type_name(ri, direction, deref=deref)} *{var}'] + self.arg_member(ri))
@@ -482,8 +497,7 @@ class TypeString(Type): 
               ['unsigned int len;']



    def _setter_lines(self, ri, member, presence):

        return [f"free({member});",

                f"{presence}_len = strlen({self.c_name});",

        return [f"{presence}_len = strlen({self.c_name});",

                f"{member} = malloc({presence}_len + 1);",

                f'memcpy({member}, {self.c_name}, {presence}_len);',

                f'{member}[{presence}_len] = 0;']
@@ -536,8 +550,7 @@ class TypeBinary(Type): 
               ['unsigned int len;']



    def _setter_lines(self, ri, member, presence):

        return [f"free({member});",

                f"{presence}_len = len;",

        return [f"{presence}_len = len;",

                f"{member} = malloc({presence}_len);",

                f'memcpy({member}, {self.c_name}, {presence}_len);']
@@ -574,12 +587,14 @@ class TypeNest(Type): 
    def _complex_member_type(self, ri):

        return self.nested_struct_type



    def free(self, ri, var, ref):

    def _free_lines(self, ri, var, ref):

        lines = []

        at = '&'

        if self.is_recursive_for_op(ri):

            at = ''

            ri.cw.p(f'if ({var}->{ref}{self.c_name})')

        ri.cw.p(f'{self.nested_render_name}_free({at}{var}->{ref}{self.c_name});')

            lines += [f'if ({var}->{ref}{self.c_name})']

        lines += [f'{self.nested_render_name}_free({at}{var}->{ref}{self.c_name});']

        return lines



    def _attr_typol(self):

        return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, '
@@ -632,15 +647,19 @@ class TypeMultiAttr(Type): 
    def free_needs_iter(self):

        return 'type' not in self.attr or self.attr['type'] == 'nest'



    def free(self, ri, var, ref):

    def _free_lines(self, ri, var, ref):

        lines = []

        if self.attr['type'] in scalars:

            ri.cw.p(f"free({var}->{ref}{self.c_name});")

            lines += [f"free({var}->{ref}{self.c_name});"]

        elif 'type' not in self.attr or self.attr['type'] == 'nest':

            ri.cw.p(f"for (i = 0; i < {var}->{ref}n_{self.c_name}; i++)")

            ri.cw.p(f'{self.nested_render_name}_free(&{var}->{ref}{self.c_name}[i]);')

            ri.cw.p(f"free({var}->{ref}{self.c_name});")

            lines += [

                f"for (i = 0; i < {var}->{ref}n_{self.c_name}; i++)",

                f'{self.nested_render_name}_free(&{var}->{ref}{self.c_name}[i]);',

                f"free({var}->{ref}{self.c_name});",

            ]

        else:

            raise Exception(f"Free of MultiAttr sub-type {self.attr['type']} not supported yet")

        return lines



    def _attr_policy(self, policy):

        return self.base_type._attr_policy(policy)
@@ -654,10 +673,10 @@ class TypeMultiAttr(Type): 
    def attr_put(self, ri, var):

        if self.attr['type'] in scalars:

            put_type = self.type

            ri.cw.p(f"for (unsigned int i = 0; i < {var}->n_{self.c_name}; i++)")

            ri.cw.p(f"for (i = 0; i < {var}->n_{self.c_name}; i++)")

            ri.cw.p(f"ynl_attr_put_{put_type}(nlh, {self.enum_name}, {var}->{self.c_name}[i]);")

        elif 'type' not in self.attr or self.attr['type'] == 'nest':

            ri.cw.p(f"for (unsigned int i = 0; i < {var}->n_{self.c_name}; i++)")

            ri.cw.p(f"for (i = 0; i < {var}->n_{self.c_name}; i++)")

            self._attr_put_line(ri, var, f"{self.nested_render_name}_put(nlh, " +

                                f"{self.enum_name}, &{var}->{self.c_name}[i])")

        else:
@@ -666,8 +685,7 @@ class TypeMultiAttr(Type): 
    def _setter_lines(self, ri, member, presence):

        # For multi-attr we have a count, not presence, hack up the presence

        presence = presence[:-(len('_present.') + len(self.c_name))] + "n_" + self.c_name

        return [f"free({member});",

                f"{member} = {self.c_name};",

        return [f"{member} = {self.c_name};",

                f"{presence} = n_{self.c_name};"]
@@ -696,8 +714,11 @@ class TypeArrayNest(Type): 
    def _attr_get(self, ri, var):

        local_vars = ['const struct nlattr *attr2;']

        get_lines = [f'attr_{self.c_name} = attr;',

                     'ynl_attr_for_each_nested(attr2, attr)',

                     f'\t{var}->n_{self.c_name}++;']

                     'ynl_attr_for_each_nested(attr2, attr) {',

                     '\tif (ynl_attr_validate(yarg, attr2))',

                     '\t\treturn YNL_PARSE_CB_ERROR;',

                     f'\t{var}->n_{self.c_name}++;',

                     '}']

        return get_lines, None, local_vars
@@ -755,6 +776,7 @@ class Struct: 
        self.request = False

        self.reply = False

        self.recursive = False

        self.in_multi_val = False  # used by a MultiAttr or and legacy arrays



        self.attr_list = []

        self.attrs = dict()
@@ -1122,6 +1144,10 @@ class Family(SpecFamily): 
                    if attr in rs_members['reply']:

                        self.pure_nested_structs[nested].reply = True



                if spec.is_multi_val():

                    child = self.pure_nested_structs.get(nested)

                    child.in_multi_val = True



        self._sort_pure_types()



        # Propagate the request / reply / recursive
@@ -1136,6 +1162,8 @@ class Family(SpecFamily): 
                            struct.child_nests.update(child.child_nests)

                        child.request |= struct.request

                        child.reply |= struct.reply

                        if spec.is_multi_val():

                            child.in_multi_val = True

                if attr_set in struct.child_nests:

                    struct.recursive = True
@@ -1399,9 +1427,9 @@ class CodeWriter: 


    def write_func(self, qual_ret, name, body, args=None, local_vars=None):

        self.write_func_prot(qual_ret=qual_ret, name=name, args=args)

        self.block_start()

        self.write_func_lvar(local_vars=local_vars)



        self.block_start()

        for line in body:

            self.p(line)

        self.block_end()
@@ -1644,11 +1672,23 @@ def put_req_nested_prototype(ri, struct, suffix=';'): 




def put_req_nested(ri, struct):

    local_vars = []

    init_lines = []



    local_vars.append('struct nlattr *nest;')

    init_lines.append("nest = ynl_attr_nest_start(nlh, attr_type);")



    for _, arg in struct.member_list():

        if arg.presence_type() == 'count':

            local_vars.append('unsigned int i;')

            break



    put_req_nested_prototype(ri, struct, suffix='')

    ri.cw.block_start()

    ri.cw.write_func_lvar('struct nlattr *nest;')

    ri.cw.write_func_lvar(local_vars)



    ri.cw.p("nest = ynl_attr_nest_start(nlh, attr_type);")

    for line in init_lines:

        ri.cw.p(line)



    for _, arg in struct.member_list():

        arg.attr_put(ri, "obj")
@@ -1850,6 +1890,11 @@ def print_req(ri): 
        local_vars += ['size_t hdr_len;',

                       'void *hdr;']



    for _, attr in ri.struct["request"].member_list():

        if attr.presence_type() == 'count':

            local_vars += ['unsigned int i;']

            break



    print_prototype(ri, direction, terminate=False)

    ri.cw.block_start()

    ri.cw.write_func_lvar(local_vars)
@@ -2941,6 +2986,9 @@ def main(): 
            for attr_set, struct in parsed.pure_nested_structs.items():

                ri = RenderInfo(cw, parsed, args.mode, "", "", attr_set)

                print_type_full(ri, struct)

                if struct.request and struct.in_multi_val:

                    free_rsp_nested_prototype(ri)

                    cw.nl()



            for op_name, op in parsed.ops.items():

                cw.p(f"/* ============== {op.enum_name} ============== */")