dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.16-5.10] md: dm-zoned-target: Initialize return variable r to avoid uninitialized use
       [not found] <20250808153054.1250675-1-sashal@kernel.org>
@ 2025-08-08 15:30 ` Sasha Levin
  2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] dm-mpath: don't print the "loaded" message if registering fails Sasha Levin
  2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] dm-table: fix checking for rq stackable devices Sasha Levin
  2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2025-08-08 15:30 UTC (permalink / raw)
  To: patches, stable
  Cc: Purva Yeshi, Mikulas Patocka, Sasha Levin, agk, snitzer, dm-devel

From: Purva Yeshi <purvayeshi550@gmail.com>

[ Upstream commit 487767bff572d46f7c37ad846c4078f6d6c9cc55 ]

Fix Smatch-detected error:
drivers/md/dm-zoned-target.c:1073 dmz_iterate_devices()
error: uninitialized symbol 'r'.

Smatch detects a possible use of the uninitialized variable 'r' in
dmz_iterate_devices() because if dmz->nr_ddevs is zero, the loop is
skipped and 'r' is returned without being set, leading to undefined
behavior.

Initialize 'r' to 0 before the loop. This ensures that if there are no
devices to iterate over, the function still returns a defined value.

Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

**Backport Status: YES**

This commit should be backported to stable kernel trees. Here's my
extensive analysis:

## Analysis of the Bug

The commit fixes a genuine uninitialized variable bug in
`dmz_iterate_devices()` function in drivers/md/dm-zoned-target.c:1073.
The bug occurs when:

1. The variable `r` is declared but not initialized at line 1065
2. If `dmz->nr_ddevs` is 0, the for loop at line 1067 is never entered
3. The function returns `r` at line 1073, which contains garbage data

This is a clear programming error that can lead to undefined behavior.
The function could return any arbitrary value when there are no devices
to iterate over.

## Why This Should Be Backported

1. **Real Bug Fix**: This fixes an actual bug that can cause
   unpredictable behavior. An uninitialized return value can lead to:
   - Incorrect error handling by callers
   - Potential system instability
   - Unpredictable behavior in device mapper operations

2. **Small and Contained Fix**: The fix is minimal - just initializing
   `r` to 0. This follows the pattern used in other similar
   iterate_devices implementations like `dm-cache-target.c:3438` where
   `int r = 0;` is properly initialized.

3. **No Side Effects**: The change has no architectural impact or side
   effects. It simply ensures the function returns a defined value (0)
   when there are no devices to iterate, which is the expected behavior.

4. **Affects Users**: While the condition (nr_ddevs == 0) might be rare,
   it's still a possible configuration that could affect users of dm-
   zoned devices. The undefined behavior could manifest in various ways
   depending on what's in memory.

5. **Static Analysis Finding**: This was caught by Smatch static
   analysis, indicating it's a real code quality issue that should be
   fixed.

6. **Follows Stable Rules**: The fix is:
   - Obviously correct
   - Fixes a real bug (uninitialized variable use)
   - Minimal risk of regression
   - Small change (1 line)
   - No new features or architectural changes

## Technical Context

Looking at the code structure, `nr_ddevs` is set from `argc` in line
858, which must be at least 1 based on the check at line 835. However,
there could be error paths or future code changes that might result in
this function being called with nr_ddevs == 0, making this defensive
programming important.

The fix aligns with how other device mapper targets handle their
iterate_devices functions, where the return value is properly
initialized before use.

 drivers/md/dm-zoned-target.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 5da3db06da10..9da329078ea4 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -1062,7 +1062,7 @@ static int dmz_iterate_devices(struct dm_target *ti,
 	struct dmz_target *dmz = ti->private;
 	unsigned int zone_nr_sectors = dmz_zone_nr_sectors(dmz->metadata);
 	sector_t capacity;
-	int i, r;
+	int i, r = 0;
 
 	for (i = 0; i < dmz->nr_ddevs; i++) {
 		capacity = dmz->dev[i].capacity & ~(zone_nr_sectors - 1);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH AUTOSEL 6.16-5.10] dm-mpath: don't print the "loaded" message if registering fails
       [not found] <20250808153054.1250675-1-sashal@kernel.org>
  2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] md: dm-zoned-target: Initialize return variable r to avoid uninitialized use Sasha Levin
@ 2025-08-08 15:30 ` Sasha Levin
  2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] dm-table: fix checking for rq stackable devices Sasha Levin
  2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2025-08-08 15:30 UTC (permalink / raw)
  To: patches, stable; +Cc: Mikulas Patocka, Sasha Levin, agk, snitzer, dm-devel

From: Mikulas Patocka <mpatocka@redhat.com>

[ Upstream commit 6e11952a6abc4641dc8ae63f01b318b31b44e8db ]

If dm_register_path_selector, don't print the "version X loaded" message.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

**Backport Status: YES**

This commit is a clear candidate for backporting to stable kernel trees.
Here's my extensive analysis:

## Bug Analysis

The commit fixes a logic error in the error handling of four device-
mapper path selector modules. Looking at the code changes:

1. **Original buggy behavior**: When `dm_register_path_selector()` fails
   (returns r < 0), the code would:
   - Print "register failed" error message
   - **Continue to print "version X loaded" message**
   - Return the error code

2. **Fixed behavior**: When registration fails:
   - Print "register failed" error message
   - **Return immediately with the error code**
   - Skip the misleading "loaded" message

## Why This Should Be Backported

1. **Clear Bug Fix**: This is a straightforward bug fix that corrects
   misleading log messages. When module registration fails, it
   incorrectly reports success with "version X loaded".

2. **Small and Contained**: The fix is minimal - just adding an early
   return statement in four similar init functions:
   - `dm_hst_init()` in dm-ps-historical-service-time.c
   - `dm_ql_init()` in dm-ps-queue-length.c
   - `dm_rr_init()` in dm-ps-round-robin.c
   - `dm_st_init()` in dm-ps-service-time.c

3. **No Side Effects**: The change only affects error path behavior and
   logging. It doesn't introduce new functionality or change any
   successful operation paths.

4. **User-Visible Issue**: This fixes a user-visible problem where
   system logs would incorrectly indicate a module loaded successfully
   when it actually failed.

5. **Consistent Pattern**: All four files have the identical bug and
   identical fix, showing this is a systematic issue that should be
   addressed.

6. **Low Risk**: The change is trivial - adding an early return on
   error. There's virtually no risk of regression since:
   - It only changes behavior when registration already failed
   - The function already returns the error code at the end
   - This just makes it return earlier without printing the misleading
     message

7. **Affects Core Subsystem**: Device-mapper is a critical storage
   subsystem, and accurate error reporting is important for system
   administrators debugging storage issues.

The commit perfectly fits stable kernel criteria: it's a small, obvious
fix for a real bug that affects users (through misleading log messages),
with minimal risk of introducing new problems.

 drivers/md/dm-ps-historical-service-time.c | 4 +++-
 drivers/md/dm-ps-queue-length.c            | 4 +++-
 drivers/md/dm-ps-round-robin.c             | 4 +++-
 drivers/md/dm-ps-service-time.c            | 4 +++-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-ps-historical-service-time.c b/drivers/md/dm-ps-historical-service-time.c
index b49e10d76d03..2c8626a83de4 100644
--- a/drivers/md/dm-ps-historical-service-time.c
+++ b/drivers/md/dm-ps-historical-service-time.c
@@ -541,8 +541,10 @@ static int __init dm_hst_init(void)
 {
 	int r = dm_register_path_selector(&hst_ps);
 
-	if (r < 0)
+	if (r < 0) {
 		DMERR("register failed %d", r);
+		return r;
+	}
 
 	DMINFO("version " HST_VERSION " loaded");
 
diff --git a/drivers/md/dm-ps-queue-length.c b/drivers/md/dm-ps-queue-length.c
index e305f05ad1e5..eb543e6431e0 100644
--- a/drivers/md/dm-ps-queue-length.c
+++ b/drivers/md/dm-ps-queue-length.c
@@ -260,8 +260,10 @@ static int __init dm_ql_init(void)
 {
 	int r = dm_register_path_selector(&ql_ps);
 
-	if (r < 0)
+	if (r < 0) {
 		DMERR("register failed %d", r);
+		return r;
+	}
 
 	DMINFO("version " QL_VERSION " loaded");
 
diff --git a/drivers/md/dm-ps-round-robin.c b/drivers/md/dm-ps-round-robin.c
index d1745b123dc1..66a15ac0c22c 100644
--- a/drivers/md/dm-ps-round-robin.c
+++ b/drivers/md/dm-ps-round-robin.c
@@ -220,8 +220,10 @@ static int __init dm_rr_init(void)
 {
 	int r = dm_register_path_selector(&rr_ps);
 
-	if (r < 0)
+	if (r < 0) {
 		DMERR("register failed %d", r);
+		return r;
+	}
 
 	DMINFO("version " RR_VERSION " loaded");
 
diff --git a/drivers/md/dm-ps-service-time.c b/drivers/md/dm-ps-service-time.c
index 969d31c40272..f8c43aecdb27 100644
--- a/drivers/md/dm-ps-service-time.c
+++ b/drivers/md/dm-ps-service-time.c
@@ -341,8 +341,10 @@ static int __init dm_st_init(void)
 {
 	int r = dm_register_path_selector(&st_ps);
 
-	if (r < 0)
+	if (r < 0) {
 		DMERR("register failed %d", r);
+		return r;
+	}
 
 	DMINFO("version " ST_VERSION " loaded");
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH AUTOSEL 6.16-6.1] dm-table: fix checking for rq stackable devices
       [not found] <20250808153054.1250675-1-sashal@kernel.org>
  2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] md: dm-zoned-target: Initialize return variable r to avoid uninitialized use Sasha Levin
  2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] dm-mpath: don't print the "loaded" message if registering fails Sasha Levin
@ 2025-08-08 15:30 ` Sasha Levin
  2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2025-08-08 15:30 UTC (permalink / raw)
  To: patches, stable
  Cc: Benjamin Marzinski, Mike Snitzer, Mikulas Patocka, Sasha Levin,
	agk, dm-devel

From: Benjamin Marzinski <bmarzins@redhat.com>

[ Upstream commit 8ca719b81987be690f197e82fdb030580c0a07f3 ]

Due to the semantics of iterate_devices(), the current code allows a
request-based dm table as long as it includes one request-stackable
device. It is supposed to only allow tables where there are no
non-request-stackable devices.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Based on my analysis of the commit and the code context, here's my
assessment:

**Backport Status: YES**

This commit fixes a clear logic bug in the device mapper table
validation code that could lead to incorrect device stacking
configurations. Here's the detailed analysis:

**1. The Bug:**
The original code had inverted logic when checking if devices are
request-stackable:
- The function `device_is_rq_stackable()` returned `true` (non-zero) for
  stackable devices
- But `iterate_devices()` stops iteration on non-zero returns and
  returns that non-zero value
- The check `!ti->type->iterate_devices(ti, device_is_rq_stackable,
  NULL)` would pass if ANY device returned true (stackable), not if ALL
  devices were stackable

**2. The Fix:**
- Renamed function to `device_is_not_rq_stackable()` to clarify its
  purpose
- Inverted the return values: returns `true` for non-stackable devices
- Changed the logic check to `ti->type->iterate_devices(ti,
  device_is_not_rq_stackable, NULL)` without negation
- Now correctly fails if ANY device is not stackable

**3. Why This Should Be Backported:**

a) **Fixes a real bug**: The current code allows invalid device stacking
configurations where request-based DM tables could include non-request-
stackable devices, potentially leading to:
   - Data corruption
   - System crashes
   - I/O failures

b) **Small and contained fix**: The change is minimal (4 lines changed)
and only affects the validation logic without changing any core
functionality

c) **Clear semantics**: The fix makes the code's intent clearer by
renaming the function to match what it actually checks

d) **No architectural changes**: This is purely a bug fix that corrects
validation logic

e) **Critical subsystem**: Device mapper is a critical component used in
production environments for LVM, encryption, and other storage features

f) **Follows stable rules**: This is exactly the type of fix stable
kernels want - important bug fixes with minimal risk

The commit message clearly explains the bug: "the current code allows a
request-based dm table as long as it includes one request-stackable
device. It is supposed to only allow tables where there are no non-
request-stackable devices." This could lead to serious issues in
production systems using device mapper.

 drivers/md/dm-table.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 24a857ff6d0b..79ba4bacd0f9 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -899,17 +899,17 @@ static bool dm_table_supports_dax(struct dm_table *t,
 	return true;
 }
 
-static int device_is_rq_stackable(struct dm_target *ti, struct dm_dev *dev,
-				  sector_t start, sector_t len, void *data)
+static int device_is_not_rq_stackable(struct dm_target *ti, struct dm_dev *dev,
+				      sector_t start, sector_t len, void *data)
 {
 	struct block_device *bdev = dev->bdev;
 	struct request_queue *q = bdev_get_queue(bdev);
 
 	/* request-based cannot stack on partitions! */
 	if (bdev_is_partition(bdev))
-		return false;
+		return true;
 
-	return queue_is_mq(q);
+	return !queue_is_mq(q);
 }
 
 static int dm_table_determine_type(struct dm_table *t)
@@ -1005,7 +1005,7 @@ static int dm_table_determine_type(struct dm_table *t)
 
 	/* Non-request-stackable devices can't be used for request-based dm */
 	if (!ti->type->iterate_devices ||
-	    !ti->type->iterate_devices(ti, device_is_rq_stackable, NULL)) {
+	    ti->type->iterate_devices(ti, device_is_not_rq_stackable, NULL)) {
 		DMERR("table load rejected: including non-request-stackable devices");
 		return -EINVAL;
 	}
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-08-08 15:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250808153054.1250675-1-sashal@kernel.org>
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] md: dm-zoned-target: Initialize return variable r to avoid uninitialized use Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] dm-mpath: don't print the "loaded" message if registering fails Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] dm-table: fix checking for rq stackable devices Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).