[cocci] [PATCH v5 4/4] coccinelle: Add kmalloc_objs conversion script

[cocci] [PATCH v5 4/4] coccinelle: Add kmalloc_objs conversion script

[Kees Cook]

Finds and converts sized kmalloc-family of allocations into the
typed kmalloc_obj-family of allocations.

Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Julia Lawall <Julia.Lawall@inria.fr>
Cc: Nicolas Palix <nicolas.palix@imag.fr>
Cc: cocci@inria.fr
---
 scripts/coccinelle/api/kmalloc_objs.cocci | 168 ++++++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100644 scripts/coccinelle/api/kmalloc_objs.cocci

diff --git a/scripts/coccinelle/api/kmalloc_objs.cocci b/scripts/coccinelle/api/kmalloc_objs.cocci
new file mode 100644
index 000000000000..39f82f014b17
--- /dev/null
+++ b/scripts/coccinelle/api/kmalloc_objs.cocci
@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/// Use kmalloc_obj family of macros for allocations
+///
+// Confidence: High
+// Comments:
+// Options: --include-headers-for-types --all-includes --include-headers --keep-comments
+
+virtual patch
+
+@initialize:python@
+@@
+import sys
+
+def alloc_array(name):
+	func = "FAILED_RENAME"
+	if name == "kmalloc_array":
+		func = "kmalloc_objs"
+	elif name == "kvmalloc_array":
+		func = "kvmalloc_objs"
+	elif name == "kcalloc":
+		func = "kzalloc_objs"
+	elif name == "kvcalloc":
+		func = "kvzalloc_objs"
+	else:
+		print(f"Unknown transform for {name}", file=sys.stderr)
+	return func
+
+@assign_sizeof depends on patch && !(file in "tools") && !(file in "samples")@
+type TYPE;
+TYPE *P;
+TYPE INST;
+expression VAR;
+expression GFP;
+expression SIZE;
+identifier ALLOC =~ "^kv?[mz]alloc$";
+fresh identifier ALLOC_OBJ_SZ = ALLOC ## "_obj_sz";
+@@
+
+(
+-	SIZE = sizeof(*VAR);
+	... when != SIZE
+	VAR =
+-	ALLOC(SIZE, GFP);
++	ALLOC_OBJ_SZ(*VAR, GFP, &SIZE);
+|
+-	SIZE = (sizeof(TYPE));
+	... when != SIZE
+	P =
+-	ALLOC(SIZE, GFP);
++	ALLOC_OBJ_SZ(*P, GFP, &SIZE);
+|
+-	SIZE = (sizeof(INST));
+	... when != SIZE
+	P =
+-	ALLOC(SIZE, GFP);
++	ALLOC_OBJ_SZ(*P, GFP, &SIZE);
+)
+
+@assign_struct_size depends on patch && !(file in "tools") && !(file in "samples")@
+type TYPE;
+TYPE *P;
+expression VAR;
+expression GFP;
+expression SIZE;
+expression FLEX;
+expression COUNT;
+identifier ALLOC =~ "^kv?[mz]alloc$";
+fresh identifier ALLOC_FLEX_SZ = ALLOC ## "_flex_sz";
+@@
+
+(
+-	SIZE = struct_size(VAR, FLEX, COUNT);
+	... when != SIZE
+	VAR =
+-	ALLOC(SIZE, GFP);
++	ALLOC_FLEX_SZ(*VAR, FLEX, COUNT, GFP, &SIZE);
+|
+-	SIZE = struct_size_t(TYPE, FLEX, COUNT);
+	... when != SIZE
+	P =
+-	ALLOC(SIZE, GFP);
++	ALLOC_FLEX_SZ(*P, FLEX, COUNT, GFP, &SIZE);
+)
+
+// This excludes anything that is assigning to or from integral types or
+// string literals. Everything else gets the sizeof() extracted for the
+// kmalloc_obj() type/var argument. sizeof(void *) is also excluded because
+// it will need case-by-case double-checking to make sure the right type is
+// being assigned.
+@direct depends on patch && !(file in "tools") && !(file in "samples")@
+typedef u8, u16, u32, u64;
+typedef __u8, __u16, __u32, __u64;
+typedef uint8_t, uint16_t, uint32_t, uint64_t;
+typedef __le16, __le32, __le64;
+typedef __be16, __be32, __be64;
+type INTEGRAL = {u8,__u8,uint8_t,char,unsigned char,
+		 u16,__u16,uint16_t,unsigned short,
+		 u32,__u32,uint32_t,unsigned int,
+		 u64,__u64,uint64_t,unsigned long,
+		 __le16,__le32,__le64,__be16,__be32,__be64};
+char [] STRING;
+INTEGRAL *BYTES;
+type TYPE;
+expression VAR;
+expression GFP;
+expression COUNT;
+expression FLEX;
+expression E;
+identifier ALLOC =~ "^kv?[mz]alloc$";
+fresh identifier ALLOC_OBJ = ALLOC ## "_obj";
+fresh identifier ALLOC_FLEX = ALLOC ## "_flex";
+identifier ALLOC_ARRAY = {kmalloc_array,kvmalloc_array,kcalloc,kvcalloc};
+fresh identifier ALLOC_OBJS = script:python(ALLOC_ARRAY) { alloc_array(ALLOC_ARRAY) };
+@@
+
+(
+-	VAR = ALLOC((sizeof(*VAR)), GFP)
++	VAR = ALLOC_OBJ(*VAR, GFP)
+|
+	ALLOC((\(sizeof(STRING)\|sizeof(INTEGRAL)\|sizeof(INTEGRAL *)\)), GFP)
+|
+	BYTES = ALLOC((sizeof(E)), GFP)
+|
+	BYTES = ALLOC((sizeof(TYPE)), GFP)
+|
+	ALLOC((sizeof(void *)), GFP)
+|
+-	ALLOC((sizeof(E)), GFP)
++	ALLOC_OBJ(E, GFP)
+|
+-	ALLOC((sizeof(TYPE)), GFP)
++	ALLOC_OBJ(TYPE, GFP)
+|
+	ALLOC_ARRAY(COUNT, (\(sizeof(STRING)\|sizeof(INTEGRAL)\|sizeof(INTEGRAL *)\)), GFP)
+|
+	BYTES = ALLOC_ARRAY(COUNT, (sizeof(E)), GFP)
+|
+	BYTES = ALLOC_ARRAY(COUNT, (sizeof(TYPE)), GFP)
+|
+	ALLOC_ARRAY((\(sizeof(STRING)\|sizeof(INTEGRAL)\|sizeof(INTEGRAL *)\)), COUNT, GFP)
+|
+	BYTES = ALLOC_ARRAY((sizeof(E)), COUNT, GFP)
+|
+	BYTES = ALLOC_ARRAY((sizeof(TYPE)), COUNT, GFP)
+|
+	ALLOC_ARRAY(COUNT, (sizeof(void *)), GFP)
+|
+	ALLOC_ARRAY((sizeof(void *)), COUNT, GFP)
+|
+-	ALLOC_ARRAY(COUNT, (sizeof(E)), GFP)
++	ALLOC_OBJS(E, COUNT, GFP)
+|
+-	ALLOC_ARRAY(COUNT, (sizeof(TYPE)), GFP)
++	ALLOC_OBJS(TYPE, COUNT, GFP)
+|
+-	ALLOC_ARRAY((sizeof(E)), COUNT, GFP)
++	ALLOC_OBJS(E, COUNT, GFP)
+|
+-	ALLOC_ARRAY((sizeof(TYPE)), COUNT, GFP)
++	ALLOC_OBJS(TYPE, COUNT, GFP)
+|
+-	ALLOC(struct_size(VAR, FLEX, COUNT), GFP)
++	ALLOC_FLEX(*VAR, FLEX, COUNT, GFP)
+|
+-	ALLOC(struct_size_t(TYPE, FLEX, COUNT), GFP)
++	ALLOC_FLEX(TYPE, FLEX, COUNT, GFP)
+)
+
-- 
2.34.1

Re: [cocci] [PATCH v5 4/4] coccinelle: Add kmalloc_objs conversion script

[Markus Elfring]

> Finds and converts sized kmalloc-family of allocations into the
> typed kmalloc_obj-family of allocations.

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc7#n94


…
> +++ b/scripts/coccinelle/api/kmalloc_objs.cocci
> @@ -0,0 +1,168 @@
…
> +// Comments:
…

* Please omit such an empty information line.

* Would a field like “Keywords” become helpful?


> +virtual patch

Will additional operation modes become relevant after clarification of implementation details?


…
> +def alloc_array(name):
> +	func = "FAILED_RENAME"
> +	if name == "kmalloc_array":
> +		func = "kmalloc_objs"
…

* I suggest to avoid duplicate variable assignments.

* How do you think about to collaborate with the Python data structure “dictionary”?


…
> +type TYPE;
> +TYPE *P;
> +TYPE INST;
> +expression VAR;
> +expression GFP;
…

Such repetition of SmPL key words can eventually be also avoided.

Regards,
Markus

Re: [cocci] [PATCH v5 4/4] coccinelle: Add kmalloc_objs conversion script

[Markus Elfring]

>> See also:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc7#n94
> 
> "Describe your changes in imperative mood"
> 
> I guess I can try rewording this, …

I became curious once more how another improved change descrition can look like.


>>> +virtual patch
>>
>> Will additional operation modes become relevant after clarification of implementation details?
> 
> I am not interested in other modes; if people want to contribute them,
> they are welcome. :)

Will development interests eventually grow accordingly?


>> …
>>> +def alloc_array(name):
>>> +	func = "FAILED_RENAME"
>>> +	if name == "kmalloc_array":
>>> +		func = "kmalloc_objs"
>> …
>>
>> * I suggest to avoid duplicate variable assignments.
>>
>> * How do you think about to collaborate with the Python data structure “dictionary”?
> 
> I wanted the stderr warning when there was no match, but to return
> "FAILED_RENAME" in such a case.

Such a data processing result can be achieved also by a more appropriate algorithm,
can't it?


>                                 Using a dictionary was basically the
> same length as the elif stack. If you have an alternative, I'm happy to
> replace it with what you come up with.

Is there a need for further case distinctions?


>> …
>>> +type TYPE;
>>> +TYPE *P;
>>> +TYPE INST;
>>> +expression VAR;
>>> +expression GFP;
>> …
>>
>> Such repetition of SmPL key words can eventually be also avoided.
> 
> That is true, but I like having them separated -- I find it more
> readable that way.

Under which circumstances would you become used also to more succinct SmPL code?

Regards,
Markus