netdev - Re: [PATCH net-next 2/5] tools: ynl-gen: allow noncontiguous enums

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <rgyzpltlodolcgr4pthrq3r2w7s5lj2mutenvszcirpwttmvjq@4dowixxyqxiw>
Date: Sun, 4 May 2025 20:25:37 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com, 
	pabeni@...hat.com, saeedm@...dia.com, horms@...nel.org, donald.hunter@...il.com
Subject: Re: [PATCH net-next 2/5] tools: ynl-gen: allow noncontiguous enums

Sat, May 03, 2025 at 03:43:47AM +0200, kuba@...nel.org wrote:
>On Fri,  2 May 2025 13:38:18 +0200 Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@...dia.com>
>> 
>> In case the enum has holes, instead of hard stop, generate a validation
>> callback to check valid enum values.
>> 
>> Signed-off-by: Jiri Pirko <jiri@...dia.com>
>> ---
>> Saeed's v3->v1:
>> - add validation callback generation
>> ---
>>  tools/net/ynl/pyynl/ynl_gen_c.py | 45 +++++++++++++++++++++++++++++---
>>  1 file changed, 42 insertions(+), 3 deletions(-)
>> 
>> diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
>> index b4889974f645..c37551473923 100755
>> --- a/tools/net/ynl/pyynl/ynl_gen_c.py
>> +++ b/tools/net/ynl/pyynl/ynl_gen_c.py
>> @@ -358,11 +358,13 @@ class TypeScalar(Type):
>>          if 'enum' in self.attr:
>>              enum = self.family.consts[self.attr['enum']]
>>              low, high = enum.value_range()
>> -            if 'min' not in self.checks:
>> +            if low and 'min' not in self.checks:
>>                  if low != 0 or self.type[0] == 's':
>>                      self.checks['min'] = low
>> -            if 'max' not in self.checks:
>> +            if high and 'max' not in self.checks:
>>                  self.checks['max'] = high
>> +            if not low and not high:
>> +                self.checks['sparse'] = True
>
>you should probably explicitly check for None, 0 is a valid low / high

Okay.


>  
>>          if 'min' in self.checks and 'max' in self.checks:
>>              if self.get_limit('min') > self.get_limit('max'):
>
>> +def print_kernel_policy_sparse_enum_validates(family, cw):
>> +    first = True
>> +    for _, attr_set in family.attr_sets.items():
>> +        if attr_set.subset_of:
>> +            continue
>> +
>> +        for _, attr in attr_set.items():
>> +            if not attr.request:
>> +                continue
>> +            if not attr.enum_name:
>> +                continue
>> +            if 'sparse' not in attr.checks:
>> +                continue
>> +
>> +            if first:
>> +                cw.p('/* Sparse enums validation callbacks */')
>> +                first = False
>> +
>> +            sign = '' if attr.type[0] == 'u' else '_signed'
>> +            suffix = 'ULL' if attr.type[0] == 'u' else 'LL'
>> +            cw.write_func_prot('static int', f'{c_lower(attr.enum_name)}_validate',
>> +                               ['const struct nlattr *attr', 'struct netlink_ext_ack *extack'])
>> +            cw.block_start()
>> +            cw.block_start(line=f'switch (nla_get_{attr["type"]}(attr))', noind=True)
>> +            enum = family.consts[attr['enum']]
>> +            for entry in enum.entries.values():
>> +                cw.p(f'case {entry.c_name}: return 0;')
>
>All the cases end in "return 0;"
>remove this, and add the return 0; before the block end.
>The code should look something like:
>
>	switch (nla_get_...) {
>	case VAL1:
>	case VAL2:
>	case VAL3:
>		return 0;
>	}

Sure.

Thanks!

>
>> +            cw.block_end(noind=True)
>> +            cw.p('NL_SET_ERR_MSG_ATTR(extack, attr, "invalid enum value");')
>> +            cw.p('return -EINVAL;')
>> +            cw.block_end()
>> +            cw.nl()
>> +
>> +
>>  def print_kernel_op_table_fwd(family, cw, terminate):
>>      exported = not kernel_can_gen_family_struct(family)
>>  
>> @@ -2965,6 +3003,7 @@ def main():
>>              print_kernel_family_struct_hdr(parsed, cw)
>>          else:
>>              print_kernel_policy_ranges(parsed, cw)
>> +            print_kernel_policy_sparse_enum_validates(parsed, cw)
>>  
>>              for _, struct in sorted(parsed.pure_nested_structs.items()):
>>                  if struct.request:
>