[PATCH] allocation: Track the size of allocated elements

[PATCH] allocation: Track the size of allocated elements

[Christophe JAILLET]

Add a new parameter, 'elem_size', which tells which argument holds the
size of an individual element.

Many checks need this parameter, and up to now, we only have an expression
that shows how to compute the total amount of memory that is allocated.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Not sure it is the best way to do it.

In smatch.h, how are populated:
 	struct expression *total_size;
 	struct expression *nr_elems;
 	struct expression *elem_size;
?

With my first trials, these expr are always NULL.
---
 smatch.h                 |  3 ++-
 smatch_allocations.c     | 26 +++++++++++++++++---------
 smatch_buf_comparison2.c |  2 +-
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/smatch.h b/smatch.h
index 00a7563185e6..c6c53c2affec 100644
--- a/smatch.h
+++ b/smatch.h
@@ -175,7 +175,8 @@ void call_string_hooks(struct string_hook_list *list, struct expression *expr, c
 
 struct allocation_info {
 	const char *fn_name;
-	const char *size_str;
+	const char *tot_size_str;
+	const char *elem_size_str;
 	struct expression *total_size;
 	struct expression *nr_elems;
 	struct expression *elem_size;
diff --git a/smatch_allocations.c b/smatch_allocations.c
index 68ca6595a8a8..81da4707d8fe 100644
--- a/smatch_allocations.c
+++ b/smatch_allocations.c
@@ -28,13 +28,14 @@ static struct alloc_hook_list *hook_funcs;
 
 struct alloc_fn_info {
 	const char *name;
-	const char *size;
+	const char *tot_size;
+	const char *elem_size;	/* if unset, assume it is the same as tot_size */
 	bool zeroed;
 };
 
 static struct alloc_fn_info alloc_fns[] = {
 	{"malloc", "$0"},
-	{"calloc", "$0 * $1", .zeroed=true},
+	{"calloc", "$0 * $1", .elem_size="$1", .zeroed=true},
 	{"memdup", "$1"},
 	{"realloc", "$1"},
 	{ },
@@ -45,8 +46,8 @@ static struct alloc_fn_info kernel_alloc_funcs[] = {
 
 	{"devm_kmalloc", "$1"},
 	{"devm_kzalloc", "$1", .zeroed=true},
-	{"devm_kmalloc_array", "$1 * $2"},
-	{"devm_kcalloc", "$1 * $2", .zeroed=true},
+	{"devm_kmalloc_array", "$1 * $2", .elem_size="$2"},
+	{"devm_kcalloc", "$1 * $2", .elem_size="$2", .zeroed=true},
 
 	{"dma_alloc_attrs", "$1", .zeroed=true},
 	{"dma_alloc_coherent", "$1", .zeroed=true},
@@ -59,18 +60,18 @@ static struct alloc_fn_info kernel_alloc_funcs[] = {
 	{"kzalloc", "$0", .zeroed=true},
 	{"kzalloc_node", "$0", .zeroed=true},
 
-	{"kmalloc_array", "$0 * $1"},
-	{"kcalloc", "$0 * $1", .zeroed=true},
+	{"kmalloc_array", "$0 * $1", .elem_size="$1"},
+	{"kcalloc", "$0 * $1", .elem_size="$1", .zeroed=true},
 
 	{"vmalloc", "$0"},
 	{"__vmalloc", "$0"},
 	{"vzalloc", "$0", .zeroed=true},
 
 	{"kvmalloc", "$0"},
-	{"kvmalloc_array", "$0 * $1"},
+	{"kvmalloc_array", "$0 * $1", .elem_size="$1"},
 	{"kvmalloc_node", "$0"},
 	{"kvzalloc", "$0", .zeroed=true},
-	{"kvcalloc", "$0 * $1", .zeroed=true},
+	{"kvcalloc", "$0 * $1", .elem_size="$1", .zeroed=true},
 	{"kvzalloc_node", "$0", .zeroed=true},
 	{"kvrealloc", "$2"},
 
@@ -102,7 +103,14 @@ static void match_alloc(struct expression *expr, const char *name, struct symbol
 	alloc_hook *fn;
 
 	data.fn_name = info->name;
-	data.size_str = info->size;
+
+	data.tot_size_str = info->tot_size;
+
+	if (info->elem_size)
+		data.elem_size_str = info->elem_size;
+	else
+		data.elem_size_str = info->tot_size;
+
 	data.zeroed = info->zeroed;
 
 	FOR_EACH_PTR(hook_funcs, fn) {
diff --git a/smatch_buf_comparison2.c b/smatch_buf_comparison2.c
index 66fb5c4f4a95..bb6f60693207 100644
--- a/smatch_buf_comparison2.c
+++ b/smatch_buf_comparison2.c
@@ -83,7 +83,7 @@ static void match_allocation(struct expression *expr,
 	sval_t sval;
 
 	/* FIXME: hack for testing */
-	if (strcmp(info->size_str, "$0") != 0)
+	if (strcmp(info->tot_size_str, "$0") != 0)
 		return;
 
 	if (expr->type != EXPR_ASSIGNMENT || expr->op != '=')
-- 
2.34.1

Re: [PATCH] allocation: Track the size of allocated elements

[Dan Carpenter]

On Thu, Aug 25, 2022 at 07:54:20PM +0200, Christophe JAILLET wrote:
> Add a new parameter, 'elem_size', which tells which argument holds the
> size of an individual element.
> 
> Many checks need this parameter, and up to now, we only have an expression
> that shows how to compute the total amount of memory that is allocated.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Not sure it is the best way to do it.
> 
> In smatch.h, how are populated:
>  	struct expression *total_size;
>  	struct expression *nr_elems;
>  	struct expression *elem_size;
> ?
> 
> With my first trials, these expr are always NULL.

Yeah.  That code is not implemented yet.  Or more accurately, I
implemented it last week but I haven't tested it yet or published it.
I think once I publish that (later today probably) then it's a better
option than passing the strings.

I guess what I'm thinking there is that I quite like the size_str the
way it is.  It's easier to fill in the alloc_fns[] table with just the
size_str.  It's ultimate in flexibility.

Then I think for kmalloc() I'll fill in the ->total_size expression.
For kmalloc_array(), I'll leave total_size NULL and fill in the
->nr_elements and ->elem_size.  It feels like there should be another
way set of expressions for struct_size().

That's sort of roughly where I'd like to go.

If you really want this then I can apply it but hopefully you won't
want it if you have the expressions instead.  Expression pointers are
better.

regards,
dan carpenter

Re: [PATCH] allocation: Track the size of allocated elements

[Marion & Christophe JAILLET]

Le 26/08/2022 à 11:31, Dan Carpenter a écrit :
> On Thu, Aug 25, 2022 at 07:54:20PM +0200, Christophe JAILLET wrote:
>> Add a new parameter, 'elem_size', which tells which argument holds the
>> size of an individual element.
>>
>> Many checks need this parameter, and up to now, we only have an expression
>> that shows how to compute the total amount of memory that is allocated.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> Not sure it is the best way to do it.
>>
>> In smatch.h, how are populated:
>>   	struct expression *total_size;
>>   	struct expression *nr_elems;
>>   	struct expression *elem_size;
>> ?
>>
>> With my first trials, these expr are always NULL.
> Yeah.  That code is not implemented yet.  Or more accurately, I
> implemented it last week but I haven't tested it yet or published it.
> I think once I publish that (later today probably) then it's a better
> option than passing the strings.
>
> I guess what I'm thinking there is that I quite like the size_str the
> way it is.  It's easier to fill in the alloc_fns[] table with just the
> size_str.  It's ultimate in flexibility.
>
> Then I think for kmalloc() I'll fill in the ->total_size expression.
> For kmalloc_array(), I'll leave total_size NULL and fill in the
> ->nr_elements and ->elem_size.  It feels like there should be another
> way set of expressions for struct_size().

Perfect for me :)

Thanks Dan.

> That's sort of roughly where I'd like to go.
>
> If you really want this then I can apply it but hopefully you won't
> want it if you have the expressions instead.  Expression pointers are
> better.

No need for my patch, I 200% agree with you.

CJ

>
> regards,
> dan carpenter
>

Re: [PATCH] allocation: Track the size of allocated elements

[Dan Carpenter]

Great!  I've applied your other patches and expression stuff and pushed
everything.

regards,
dan carpenter