| 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
| ||
|
Message-ID: <20250414211851.602096-4-kuba@kernel.org>
Date: Mon, 14 Apr 2025 14:18:46 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: davem@...emloft.net
Cc: donald.hunter@...il.com,
netdev@...r.kernel.org,
edumazet@...gle.com,
pabeni@...hat.com,
andrew+netdev@...n.ch,
horms@...nel.org,
daniel@...earbox.net,
sdf@...ichev.me,
jacob.e.keller@...el.com,
Jakub Kicinski <kuba@...nel.org>
Subject: [PATCH net 3/8] tools: ynl-gen: individually free previous values on double set
When user calls request_attrA_set() multiple times (for the same
attribute), and attrA is of type which allocates memory -
we try to free the previously associated values. For array
types (including multi-attr) we have only freed the array,
but the array may have contained pointers.
Refactor the code generation for free attr and reuse the generated
lines in setters to flush out the previous state. Since setters
are static inlines in the header we need to add forward declarations
for the free helpers of pure nested structs. Track which types get
used by arrays and include the right forwad declarations.
At least ethtool string set and bit set would not be freed without
this. Tho, admittedly, overriding already set attribute twice is likely
a very very rare thing to do.
Fixes: be5bea1cc0bf ("net: add basic C code generators for Netlink")
Signed-off-by: Jakub Kicinski <kuba@...nel.org>
---
tools/net/ynl/pyynl/ynl_gen_c.py | 62 +++++++++++++++++++++++---------
1 file changed, 45 insertions(+), 17 deletions(-)
diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index 662a925bd9e1..2d856ccc88f4 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py
@@ -162,9 +162,15 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
['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 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
['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 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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)
@@ -666,8 +685,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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};"]
@@ -755,6 +773,7 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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 +1141,10 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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 +1159,8 @@ from lib import SpecFamily, SpecAttrSet, SpecAttr, SpecOperation, SpecEnumSet, S
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
@@ -2958,6 +2983,9 @@ _C_KW = {
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} ============== */")
--
2.49.0
Powered by blists - more mailing lists