[PATCH v1 0/3] drivers/soundwire: fix memory safety issues

[PATCH v1 0/3] drivers/soundwire: fix memory safety issues

[Baoli.Zhang]

This patch series fixes three memory-safety issues in
sdw_add_element_group_count() in drivers/soundwire.

1. Fix out-of-bounds memory access caused by incorrect boundary checks.
2. Increase group->max_size only after successful allocation to avoid
   leaving the group's state inconsistent if one allocation fails.
3. Use krealloc_array() to prevent integer overflow.

Baoli.Zhang (3):
  soundwire: fix bug in sdw_add_element_group_count found by syzkaller
  soundwire: increase group->max_size after allocation
  soundwire: use krealloc_array to prevent integer overflow

 .../soundwire/generic_bandwidth_allocation.c  | 44 +++++++++----------
 1 file changed, 20 insertions(+), 24 deletions(-)

-- 
2.43.0

[PATCH v1 1/3] soundwire: fix bug in sdw_add_element_group_count found by syzkaller

[Baoli.Zhang]

The original implementation caused an out-of-bounds memory access
in the sdw_add_element_group_count for-loop when i == num.

for (i = 0; i <= num; i++) {
    if (rate == group->rates[i] && lane == group->lanes[i])
        ...

To fix this error, the function now checks for existing rate/lane
entries in the group(a function parameter) using a for-loop before
adding them.

No functional changes apart from this fix.

Fixes: 9026118f20e2 ("soundwire: Add generic bandwidth allocation algorithm")
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Baoli.Zhang <baoli.zhang@linux.intel.com>
---
 .../soundwire/generic_bandwidth_allocation.c  | 47 +++++++++----------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index fb3970e12dac9..f016ad088a1db 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -299,39 +299,36 @@ static int sdw_add_element_group_count(struct sdw_group *group,
 	int num = group->count;
 	int i;
 
-	for (i = 0; i <= num; i++) {
+	for (i = 0; i < num; i++) {
 		if (rate == group->rates[i] && lane == group->lanes[i])
-			break;
-
-		if (i != num)
-			continue;
-
-		if (group->count >= group->max_size) {
-			unsigned int *rates;
-			unsigned int *lanes;
+			return 0;
+	}
 
-			group->max_size += 1;
-			rates = krealloc(group->rates,
-					 (sizeof(int) * group->max_size),
-					 GFP_KERNEL);
-			if (!rates)
-				return -ENOMEM;
+	if (group->count >= group->max_size) {
+		unsigned int *rates;
+		unsigned int *lanes;
 
-			group->rates = rates;
+		group->max_size += 1;
+		rates = krealloc(group->rates,
+				 (sizeof(int) * group->max_size),
+				 GFP_KERNEL);
+		if (!rates)
+			return -ENOMEM;
 
-			lanes = krealloc(group->lanes,
-					 (sizeof(int) * group->max_size),
-					 GFP_KERNEL);
-			if (!lanes)
-				return -ENOMEM;
+		group->rates = rates;
 
-			group->lanes = lanes;
-		}
+		lanes = krealloc(group->lanes,
+				 (sizeof(int) * group->max_size),
+				 GFP_KERNEL);
+		if (!lanes)
+			return -ENOMEM;
 
-		group->rates[group->count] = rate;
-		group->lanes[group->count++] = lane;
+		group->lanes = lanes;
 	}
 
+	group->rates[group->count] = rate;
+	group->lanes[group->count++] = lane;
+
 	return 0;
 }
 
-- 
2.43.0

[PATCH v1 2/3] soundwire: increase group->max_size after allocation

[Baoli.Zhang]

Only update `group->max_size` after both allocations succeed to avoid
leaving the group's state inconsistent if one allocation fails.

Signed-off-by: Baoli.Zhang <baoli.zhang@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/soundwire/generic_bandwidth_allocation.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index f016ad088a1db..cd9ccbaf0e46f 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -308,9 +308,8 @@ static int sdw_add_element_group_count(struct sdw_group *group,
 		unsigned int *rates;
 		unsigned int *lanes;
 
-		group->max_size += 1;
 		rates = krealloc(group->rates,
-				 (sizeof(int) * group->max_size),
+				 sizeof(int) * (group->max_size + 1),
 				 GFP_KERNEL);
 		if (!rates)
 			return -ENOMEM;
@@ -318,12 +317,14 @@ static int sdw_add_element_group_count(struct sdw_group *group,
 		group->rates = rates;
 
 		lanes = krealloc(group->lanes,
-				 (sizeof(int) * group->max_size),
+				 sizeof(int) * (group->max_size + 1),
 				 GFP_KERNEL);
 		if (!lanes)
 			return -ENOMEM;
 
 		group->lanes = lanes;
+
+		group->max_size += 1;
 	}
 
 	group->rates[group->count] = rate;
-- 
2.43.0

[PATCH v1 3/3] soundwire: use krealloc_array to prevent integer overflow

[Baoli.Zhang]

Replace the use of krealloc() with krealloc_array() in
sdw_add_element_group_count to mitigate the risk of integer overflow during
memory allocation size calculation.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Baoli.Zhang <baoli.zhang@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/soundwire/generic_bandwidth_allocation.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index cd9ccbaf0e46f..3575d69ce1c50 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -308,17 +308,15 @@ static int sdw_add_element_group_count(struct sdw_group *group,
 		unsigned int *rates;
 		unsigned int *lanes;
 
-		rates = krealloc(group->rates,
-				 sizeof(int) * (group->max_size + 1),
-				 GFP_KERNEL);
+		rates = krealloc_array(group->rates, group->max_size + 1,
+				       sizeof(*group->rates), GFP_KERNEL);
 		if (!rates)
 			return -ENOMEM;
 
 		group->rates = rates;
 
-		lanes = krealloc(group->lanes,
-				 sizeof(int) * (group->max_size + 1),
-				 GFP_KERNEL);
+		lanes = krealloc_array(group->lanes, group->max_size + 1,
+				       sizeof(*group->lanes), GFP_KERNEL);
 		if (!lanes)
 			return -ENOMEM;
 
-- 
2.43.0