[PATCH] check_ida_alloc: Add a new check to spot invalid conversion from ida_simple_get()

[PATCH] check_ida_alloc: Add a new check to spot invalid conversion from ida_simple_get()

[Christophe JAILLET]

The 'max' parameter of ida_alloc_max() and ida_alloc_range() is not
interpreted as it was in the deprecated ida_simple_get().

The 'max' value in the new functions is inclusive but it was exclusive
before. So, in older code, this parameter was often a power of 2, such as
1 << 16, so that the maximum possible value was 0xffff.

Now a power of 2 value is spurious.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Most of the code is stolen from smatch_power_of_two.c
---
 check_ida_alloc.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++
 check_list.h      |  1 +
 2 files changed, 84 insertions(+)
 create mode 100644 check_ida_alloc.c

diff --git a/check_ida_alloc.c b/check_ida_alloc.c
new file mode 100644
index 000000000000..7f1f7463d404
--- /dev/null
+++ b/check_ida_alloc.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2022 Christophe Jaillet.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
+ */
+
+/*
+ * The 'max' parameter of ida_alloc_max() and ida_alloc_range() is not interpreted
+ * as it was in the deprecated ida_simple_get().
+ *
+ * The 'max' value in the new functions is inclusive but it was exclusive before.
+ * So, in older code, this parameter was often a power of 2, such as 1 << 16, so
+ * that the maximum possible value was 0xffff.
+ *
+ * Now a power of 2 value is spurious.
+ */
+#include <fcntl.h>
+#include <unistd.h>
+#include "parse.h"
+#include "smatch.h"
+
+static int my_id;
+
+static bool implied_power_of_two(struct expression *expr)
+{
+	sval_t sval;
+
+	if (!get_implied_value(expr, &sval))
+		return false;
+	if (!(sval.uvalue & (sval.uvalue - 1)))
+		return true;
+	return false;
+}
+
+static bool is_power_of_two(struct expression *expr)
+{
+	expr = strip_expr(expr);
+
+	if (expr->type == EXPR_BINOP &&
+	    expr->op == SPECIAL_LEFTSHIFT &&
+	    is_power_of_two(expr->left))
+		return true;
+
+	if (implied_power_of_two(expr))
+		return true;
+
+	return false;
+}
+
+static void match_ida_alloc(const char *fn, struct expression *expr, void *_arg_nr)
+{
+	int arg_nr = PTR_INT(_arg_nr);
+	struct expression *arg_expr;
+
+	arg_expr = get_argument_from_call_expr(expr->args, arg_nr);
+	arg_expr = strip_expr(arg_expr);
+
+	if (is_power_of_two(arg_expr))
+		sm_error("Calling %s() with a 'max' argument which is a power of 2. -1 missing?",
+			 fn);
+}
+
+void check_ida_alloc(int id)
+{
+	if (option_project != PROJ_KERNEL)
+		return;
+
+	my_id = id;
+
+	add_function_hook("ida_alloc_max", &match_ida_alloc, INT_PTR(1));
+	add_function_hook("ida_alloc_range", &match_ida_alloc, INT_PTR(2));
+}
diff --git a/check_list.h b/check_list.h
index fc6bce42656a..3f8e35d34a8b 100644
--- a/check_list.h
+++ b/check_list.h
@@ -183,6 +183,7 @@ CK(check_trinity_generator)
 CK(register_param_bits_set)
 CK(register_param_bits_clear)
 CK(check_do_while_loop_limit)
+CK(check_ida_alloc)
 
 /* <- your test goes here */
 /* CK(register_template) */
-- 
2.34.1

Re: [PATCH] check_ida_alloc: Add a new check to spot invalid conversion from ida_simple_get()

[Dan Carpenter]

On Thu, Jun 09, 2022 at 10:33:40PM +0200, Christophe JAILLET wrote:
> The 'max' parameter of ida_alloc_max() and ida_alloc_range() is not
> interpreted as it was in the deprecated ida_simple_get().
> 
> The 'max' value in the new functions is inclusive but it was exclusive
> before. So, in older code, this parameter was often a power of 2, such as
> 1 << 16, so that the maximum possible value was 0xffff.
> 
> Now a power of 2 value is spurious.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Most of the code is stolen from smatch_power_of_two.c
> ---

Thanks, applied!

regards,
dan carpenter