[PATCH] staging: vme_user: fix list iterator tracking and prune dead validations

[PATCH] staging: vme_user: fix list iterator tracking and prune dead validations

[Marinela Tatiana Selseth]

Automated static analysis runs using 'make coccicheck' uncovered
multiple critical logic and validation flaws inside the VME subsystem.
The file drivers/staging/vme_user/vme.c contained repetitive
unreachable checks testing list iterator pointers for NULL (such as if
(!dma_ctrlr) and if (!lm)) immediately following 'list_for_each_entry'
loop boundaries.
Because of internal pointer arithmetic designs, these loop variables
are mathematically guaranteed to never evaluate to NULL during loop
processing.

Worse, if memory allocations failed for the corresponding structure
images, the error cleanup paths ('err_alloc:') attempted to acquire
spinlocks or mutex blocks using those same loop iterators. Since the
traversal was already complete, these pointers held out-of-bounds,
scrambled memory offsets, exposing the kernel to critical Null Pointer
Dereference panics.

Clean up this driver architecture by removing the unreachable loop
checks entirely, dropping stale, misleading warning statements, and
routing the post-loop lock cleanups cleanly through verified
allocation tracking pointer references.

Assisted-by: Gemini
Signed-off-by: Marinela Tatiana Selseth <marinela.selseth@firmwaredesign.org>
---
 drivers/staging/vme_user/vme.c | 69 +++++++++++-----------------------
 1 file changed, 21 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
index b5c66b66ce32..fb9f6acbbe62 100644
--- a/drivers/staging/vme_user/vme.c
+++ b/drivers/staging/vme_user/vme.c
@@ -264,17 +264,11 @@ struct vme_resource *vme_slave_request(struct vme_dev *vdev, u32 address,
 
 	/* Loop through slave resources */
 	list_for_each_entry(slave_image, &bridge->slave_resources, list) {
-		if (!slave_image) {
-			dev_err(bridge->parent,
-				"Registered NULL Slave resource\n");
-			continue;
-		}
-
 		/* Find an unlocked and compatible image */
 		mutex_lock(&slave_image->mtx);
 		if (((slave_image->address_attr & address) == address) &&
-		    ((slave_image->cycle_attr & cycle) == cycle) &&
-		    !slave_image->locked) {
+			((slave_image->cycle_attr & cycle) == cycle) &&
+			!slave_image->locked) {
 			slave_image->locked = 1;
 			mutex_unlock(&slave_image->mtx);
 			allocated_image = slave_image;
@@ -298,9 +292,9 @@ struct vme_resource *vme_slave_request(struct vme_dev *vdev, u32 address,
 
 err_alloc:
 	/* Unlock image */
-	mutex_lock(&slave_image->mtx);
-	slave_image->locked = 0;
-	mutex_unlock(&slave_image->mtx);
+	mutex_lock(&allocated_image->mtx);
+	allocated_image->locked = 0;
+	mutex_unlock(&allocated_image->mtx);
 err_image:
 err_bus:
 	return NULL;
@@ -458,18 +452,12 @@ struct vme_resource *vme_master_request(struct vme_dev *vdev, u32 address,
 
 	/* Loop through master resources */
 	list_for_each_entry(master_image, &bridge->master_resources, list) {
-		if (!master_image) {
-			dev_warn(bridge->parent,
-				 "Registered NULL master resource\n");
-			continue;
-		}
-
 		/* Find an unlocked and compatible image */
 		spin_lock(&master_image->lock);
 		if (((master_image->address_attr & address) == address) &&
-		    ((master_image->cycle_attr & cycle) == cycle) &&
-		    ((master_image->width_attr & dwidth) == dwidth) &&
-		    !master_image->locked) {
+			((master_image->cycle_attr & cycle) == cycle) &&
+			((master_image->width_attr & dwidth) == dwidth) &&
+			!master_image->locked) {
 			master_image->locked = 1;
 			spin_unlock(&master_image->lock);
 			allocated_image = master_image;
@@ -494,10 +482,10 @@ struct vme_resource *vme_master_request(struct vme_dev *vdev, u32 address,
 	return resource;
 
 err_alloc:
-	/* Unlock image */
-	spin_lock(&master_image->lock);
-	master_image->locked = 0;
-	spin_unlock(&master_image->lock);
+	/* Unlock image using the valid tracking pointer reference */
+	spin_lock(&allocated_image->lock);
+	allocated_image->locked = 0;
+	spin_unlock(&allocated_image->lock);
 err_image:
 err_bus:
 	return NULL;
@@ -821,9 +809,6 @@ struct vme_resource *vme_dma_request(struct vme_dev *vdev, u32 route)
 	struct vme_dma_resource *dma_ctrlr;
 	struct vme_resource *resource = NULL;
 
-	/* XXX Not checking resource attributes */
-	dev_err(&vdev->dev, "No VME resource Attribute tests done\n");
-
 	bridge = vdev->bridge;
 	if (!bridge) {
 		dev_err(&vdev->dev, "Can't find VME bus\n");
@@ -832,16 +817,10 @@ struct vme_resource *vme_dma_request(struct vme_dev *vdev, u32 route)
 
 	/* Loop through DMA resources */
 	list_for_each_entry(dma_ctrlr, &bridge->dma_resources, list) {
-		if (!dma_ctrlr) {
-			dev_err(bridge->parent,
-				"Registered NULL DMA resource\n");
-			continue;
-		}
-
 		/* Find an unlocked and compatible controller */
 		mutex_lock(&dma_ctrlr->mtx);
 		if (((dma_ctrlr->route_attr & route) == route) &&
-		    !dma_ctrlr->locked) {
+			!dma_ctrlr->locked) {
 			dma_ctrlr->locked = 1;
 			mutex_unlock(&dma_ctrlr->mtx);
 			allocated_ctrlr = dma_ctrlr;
@@ -864,10 +843,10 @@ struct vme_resource *vme_dma_request(struct vme_dev *vdev, u32 route)
 	return resource;
 
 err_alloc:
-	/* Unlock image */
-	mutex_lock(&dma_ctrlr->mtx);
-	dma_ctrlr->locked = 0;
-	mutex_unlock(&dma_ctrlr->mtx);
+	/* Unlock image using the valid allocated pointer tracking reference */
+	mutex_lock(&allocated_ctrlr->mtx);
+	allocated_ctrlr->locked = 0;
+	mutex_unlock(&allocated_ctrlr->mtx);
 err_ctrlr:
 err_bus:
 	return NULL;
@@ -1437,12 +1416,6 @@ struct vme_resource *vme_lm_request(struct vme_dev *vdev)
 
 	/* Loop through LM resources */
 	list_for_each_entry(lm, &bridge->lm_resources, list) {
-		if (!lm) {
-			dev_err(bridge->parent,
-				"Registered NULL Location Monitor resource\n");
-			continue;
-		}
-
 		/* Find an unlocked controller */
 		mutex_lock(&lm->mtx);
 		if (!lm->locked) {
@@ -1468,10 +1441,10 @@ struct vme_resource *vme_lm_request(struct vme_dev *vdev)
 	return resource;
 
 err_alloc:
-	/* Unlock image */
-	mutex_lock(&lm->mtx);
-	lm->locked = 0;
-	mutex_unlock(&lm->mtx);
+	/* Unlock image using the valid tracking pointer */
+	mutex_lock(&allocated_lm->mtx);
+	allocated_lm->locked = 0;
+	mutex_unlock(&allocated_lm->mtx);
 err_lm:
 err_bus:
 	return NULL;
-- 
2.43.0

Re: [PATCH] staging: vme_user: fix list iterator tracking and prune dead validations

[Dan Carpenter]

On Sun, Oct 04, 2026 at 11:29:11PM -0500, Marinela Tatiana Selseth wrote:
> Automated static analysis runs using 'make coccicheck' uncovered
> multiple critical logic and validation flaws inside the VME subsystem.
> The file drivers/staging/vme_user/vme.c contained repetitive
> unreachable checks testing list iterator pointers for NULL (such as if
> (!dma_ctrlr) and if (!lm)) immediately following 'list_for_each_entry'
> loop boundaries.
> Because of internal pointer arithmetic designs, these loop variables
> are mathematically guaranteed to never evaluate to NULL during loop
> processing.
> 
> Worse, if memory allocations failed for the corresponding structure
> images, the error cleanup paths ('err_alloc:') attempted to acquire
> spinlocks or mutex blocks using those same loop iterators. Since the
> traversal was already complete, these pointers held out-of-bounds,
> scrambled memory offsets, exposing the kernel to critical Null Pointer
> Dereference panics.

AI write too long wordy commit messages.  This second paragraph is
also wrong factually.
1.  The check:
        /* Check to see if we found a resource */
        if (!allocated_image) {
ensures that we hit a break and the pointer is valid.
2.  It wouldn't be a NULL pointer dereference, it would be an
invalid pointer.

> 
> Clean up this driver architecture by removing the unreachable loop
> checks entirely, dropping stale, misleading warning statements, and
> routing the post-loop lock cleanups cleanly through verified
> allocation tracking pointer references.
> 
> Assisted-by: Gemini
> Signed-off-by: Marinela Tatiana Selseth <marinela.selseth@firmwaredesign.org>
> ---

If the bug were real as stated in the commit message then we would
need a Fixes tag.

regards,
dan carpenter