Skip to contentSkip to conversation

cciss: call BUG() earlier

PATCH In review Dan Carpenter 3 participants 4 messages Jun 1, 2010

Subsystems (2)
Block layer Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 cciss: call BUG() earlier

I moved the range check after the increment. The current code would write past the end of the array once before calling BUG().

Signed-off-by Dan Carpenter <[email protected]>
drivers/block/cciss_scsi.c +1 -1
index e1d0e2c..3381505 100644
@@ -188,11 +188,11 @@ scsi_cmd_free(ctlr_info_t *h, CommandList_struct *cmd)
 
 	sa = h->scsi_ctlr;
 	stk = &sa->cmd_stack; 
+	stk->top++;
 	if (stk->top >= CMD_STACK_SIZE) {
 		printk("cciss: scsi_cmd_free called too many times.\n");
 		BUG();
 	}
-	stk->top++;
 	stk->elem[stk->top] = (struct cciss_scsi_cmd_stack_elem_t *) cmd;
 }
#1 lore raw

Hi Dan,

Did you diff this one backwards ?

Quoted 23 lines · Dan Carpenter

I moved the range check after the increment.  The current code would write past the end of the array once before calling BUG().

Signed-off-by Dan Carpenter <[email protected]>
drivers/block/cciss_scsi.c +1 -2
index e1d0e2c..3381505 100644
@@ -188,11 +188,11 @@ scsi_cmd_free(ctlr_info_t *h, CommandList_struct *cmd)
 
        sa = h->scsi_ctlr;
        stk = &sa->cmd_stack;
+       stk->top++;
        if (stk->top >= CMD_STACK_SIZE) {
                printk("cciss: scsi_cmd_free called too many times.\n");
                BUG();
        }
-       stk->top++;
        stk->elem[stk->top] = (struct cciss_scsi_cmd_stack_elem_t *) cmd;
  }
 
--
Quoted 4 lines

To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
In reply to Dan Carpenter
Quoted 14 lines · Darren Jenkins

Hi Dan,

Quoted 11 lines · Dan Carpenter

I moved the range check after the increment.  The current code would write past the end of the array once before calling BUG().

Signed-off-by Dan Carpenter <[email protected]>
drivers/block/cciss_scsi.c +0 -0
index e1d0e2c..3381505 100644
@@ -188,11 +188,11 @@ scsi_cmd_free(ctlr_info_t *h, CommandList_struct *cmd)
Quoted 13 lines

drivers/block/cciss_scsi.c scsi_cmd_free(ctlr_info_t *h, CommandList_struct *cmd) · line 197

Quoted 10 lines
drivers/block/cciss_scsi.c +1 -1
        sa = h->scsi_ctlr;
        stk = &sa->cmd_stack;
+       stk->top++;
        if (stk->top >= CMD_STACK_SIZE) {
                printk("cciss: scsi_cmd_free called too many times.\n");
                BUG();
        }
-       stk->top++;
        stk->elem[stk->top] = (struct cciss_scsi_cmd_stack_elem_t *) cmd;
  }

Did you diff this one backwards ?

How so? if stk->top = CMD_STACK_SIZE - 1 the current code will not trigger the BUG, but it will index beyond the size of ->elem. So the patch looks correct to me.

Dan, I'll apply it, thanks.

Signature · 6 lines
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#3 lore raw
In reply to Dan Carpenter

Hi Jens

Quoted 4 lines · Jens Axboe

How so? if stk->top = CMD_STACK_SIZE - 1 the current code will not trigger the BUG, but it will index beyond the size of ->elem. So the patch looks correct to me.

I guess I was looking at the wrong thing, it does look correct. Sorry for the noise

Darren J.