[PATCH v2 0/2] kcsan: debugs: Refactor allocation code

[PATCH v2 0/2] kcsan: debugs: Refactor allocation code

[Andy Shevchenko]

Refactor allocation code to be more robust against overflows
and shorted in terms of LoCs.

In v2:
- collected tags (Marco)
- added patch 2

Andy Shevchenko (2):
  kcsan: debugfs: Use krealloc_array() to replace krealloc()
  kcsan: debugfs: Use krealloc_array() for initial allocation as well

 kernel/kcsan/debugfs.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v2 1/2] kcsan: debugfs: Use krealloc_array() to replace krealloc()

[Andy Shevchenko]

Use krealloc_array() to replace krealloc() with multiplication.
krealloc_array() has multiply overflow check, which will be safer.

Reviewed-by: Marco Elver <elver@google.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/kcsan/debugfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c
index 53b21ae30e00..be7051d0e7f4 100644
--- a/kernel/kcsan/debugfs.c
+++ b/kernel/kcsan/debugfs.c
@@ -166,10 +166,10 @@ static ssize_t insert_report_filterlist(const char *func)
 	} else if (report_filterlist.used == report_filterlist.size) {
 		/* resize filterlist */
 		size_t new_size = report_filterlist.size * 2;
-		unsigned long *new_addrs =
-			krealloc(report_filterlist.addrs,
-				 new_size * sizeof(unsigned long), GFP_ATOMIC);
+		unsigned long *new_addrs;
 
+		new_addrs = krealloc_array(report_filterlist.addrs,
+					   new_size, sizeof(*new_addrs), GFP_ATOMIC);
 		if (new_addrs == NULL) {
 			/* leave filterlist itself untouched */
 			ret = -ENOMEM;
-- 
2.43.0.rc1.1336.g36b5255a03ac

[PATCH v2 2/2] kcsan: debugfs: Use krealloc_array() for initial allocation as well

[Andy Shevchenko]

Use krealloc_array() for initial allocation as well.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/kcsan/debugfs.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c
index be7051d0e7f4..ac31412de646 100644
--- a/kernel/kcsan/debugfs.c
+++ b/kernel/kcsan/debugfs.c
@@ -145,6 +145,8 @@ static ssize_t insert_report_filterlist(const char *func)
 {
 	unsigned long flags;
 	unsigned long addr = kallsyms_lookup_name(func);
+	unsigned long *new_addrs;
+	size_t new_size = 0;
 	ssize_t ret = 0;
 
 	if (!addr) {
@@ -156,18 +158,12 @@ static ssize_t insert_report_filterlist(const char *func)
 
 	if (report_filterlist.addrs == NULL) {
 		/* initial allocation */
-		report_filterlist.addrs =
-			kmalloc_array(report_filterlist.size,
-				      sizeof(unsigned long), GFP_ATOMIC);
-		if (report_filterlist.addrs == NULL) {
-			ret = -ENOMEM;
-			goto out;
-		}
+		new_size = report_filterlist.size;
 	} else if (report_filterlist.used == report_filterlist.size) {
 		/* resize filterlist */
-		size_t new_size = report_filterlist.size * 2;
-		unsigned long *new_addrs;
-
+		new_size = report_filterlist.size * 2;
+	}
+	if (new_size) {
 		new_addrs = krealloc_array(report_filterlist.addrs,
 					   new_size, sizeof(*new_addrs), GFP_ATOMIC);
 		if (new_addrs == NULL) {
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v2 0/2] kcsan: debugs: Refactor allocation code

[Andy Shevchenko]

On Thu, Nov 21, 2024 at 04:12:50PM +0200, Andy Shevchenko wrote:
> Refactor allocation code to be more robust against overflows
> and shorted in terms of LoCs.
> 
> In v2:
> - collected tags (Marco)
> - added patch 2

Okay, it seems I have to check the Linux Next for the current state of
affairs...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] kcsan: debugs: Refactor allocation code

[Marco Elver]

On Thu, 21 Nov 2024 at 15:17, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Nov 21, 2024 at 04:12:50PM +0200, Andy Shevchenko wrote:
> > Refactor allocation code to be more robust against overflows
> > and shorted in terms of LoCs.
> >
> > In v2:
> > - collected tags (Marco)
> > - added patch 2
>
> Okay, it seems I have to check the Linux Next for the current state of
> affairs...

Right. Please double check this still applies after 59458fa4ddb4
("kcsan: Turn report_filterlist_lock into a raw_spinlock") in latest
mainline (or -next). I had to rework that code to make PREEMPT_RT
happy, and at the same time got rid of all this old code. I suppose a
side-effect was switching over to kmalloc_array() and making it look a
bit cleaner as well.

Thanks,
-- Marco