Skip to contentSkip to conversation

staging: dgnc: some off by one bugs

PATCH Superseded Dan Carpenter 1 participant 3 messages Mar 12, 2015

Subsystems (3)
Staging Kernel janitors TTY layer and serial drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/2 staging: dgnc: some off by one bugs

"dgnc_NumBoards" is the number of filled out elements in the dgnc_Board[] array. "->nasync" and "->maxports" are the value. They are the number of channels in the ->channels[] array so these tests should be ">=" instead of ">" so we avoid reading past the end of the arrays.

I cleaned up the conditions in dgnc_mgmt_ioctl() a bit. There was a work around for the off by one bug in the case where there were no boards which is no longer needed. "channel" is unsigned so it can't be negative.

Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/dgnc/dgnc_cls.c +1 -1
index bedc522..0d8f154 100644
@@ -406,7 +406,7 @@ static inline void cls_parse_isr(struct dgnc_board *brd, uint port)
 	 * verified in the interrupt routine.
 	 */
 
-	if (port > brd->nasync)
+	if (port >= brd->nasync)
 		return;
 
 	ch = brd->channels[port];
drivers/staging/dgnc/dgnc_mgmt.c +2 -2
index 5544a8e..01b290e 100644
@@ -196,11 +196,11 @@ long dgnc_mgmt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		channel = ni.channel;
 
 		/* Verify boundaries on board */
-		if ((board > dgnc_NumBoards) || (dgnc_NumBoards = 0))
+		if (board >= dgnc_NumBoards)
 			return -ENODEV;
 
 		/* Verify boundaries on channel */
-		if ((channel < 0) || (channel > dgnc_Board[board]->nasync))
+		if (channel >= dgnc_Board[board]->nasync)
 			return -ENODEV;
 
 		ch = dgnc_Board[board]->channels[channel];
drivers/staging/dgnc/dgnc_neo.c +3 -3
index 1268aa9..921e463 100644
@@ -407,7 +407,7 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
 	if (!brd || brd->magic != DGNC_BOARD_MAGIC)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];

@@ -537,7 +537,7 @@ static inline void neo_parse_lsr(struct dgnc_board *brd, uint port)
 	if (!brd || brd->magic != DGNC_BOARD_MAGIC)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];

@@ -1019,7 +1019,7 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
 			 */
 
 			/* Verify the port is in range. */
-			if (port > brd->nasync)
+			if (port >= brd->nasync)
 				continue;
 
 			ch = brd->channels[port];
drivers/staging/dgnc/dgnc_tty.c +1 -1
index 8179342..4507d4a 100644
@@ -1060,7 +1060,7 @@ static int dgnc_tty_open(struct tty_struct *tty, struct file *file)
 	spin_lock_irqsave(&brd->bd_lock, flags);
 
 	/* If opened device is greater than our number of ports, bail. */
-	if (PORT_NUM(minor) > brd->nasync) {
+	if (PORT_NUM(minor) >= brd->nasync) {
 		spin_unlock_irqrestore(&brd->bd_lock, flags);
 		return -ENXIO;
 	}
author
#1 lore raw
Quoted 2 lines · Dan Carpenter

"dgnc_NumBoards" is the number of filled out elements in the dgnc_Board[] array. "->nasync" and "->maxports" are the value.

I meant "the *same* value". I'll resend.

regards,
dan carpenter

author
#2 lore raw

PATCH v2 1/2 staging: dgnc: some off by one bugs

In reply to Dan Carpenter

"dgnc_NumBoards" is the number of filled out elements in the dgnc_Board[] array. "->nasync" and "->maxports" are the same value. They are the number of channels in the ->channels[] array so these tests should be ">=" instead of ">" so we avoid reading past the end of the arrays.

I cleaned up the conditions in dgnc_mgmt_ioctl() a bit. There was a work around for the off by one bug in the case where there were no boards which is no longer needed. "channel" is unsigned so it can't be negative.

Signed-off-by Dan Carpenter <[email protected]>

---
v2: fix changelog

drivers/staging/dgnc/dgnc_cls.c +1 -1
index bedc522..0d8f154 100644
@@ -406,7 +406,7 @@ static inline void cls_parse_isr(struct dgnc_board *brd, uint port)
 	 * verified in the interrupt routine.
 	 */
 
-	if (port > brd->nasync)
+	if (port >= brd->nasync)
 		return;
 
 	ch = brd->channels[port];
drivers/staging/dgnc/dgnc_mgmt.c +2 -2
index 5544a8e..01b290e 100644
@@ -196,11 +196,11 @@ long dgnc_mgmt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		channel = ni.channel;
 
 		/* Verify boundaries on board */
-		if ((board > dgnc_NumBoards) || (dgnc_NumBoards = 0))
+		if (board >= dgnc_NumBoards)
 			return -ENODEV;
 
 		/* Verify boundaries on channel */
-		if ((channel < 0) || (channel > dgnc_Board[board]->nasync))
+		if (channel >= dgnc_Board[board]->nasync)
 			return -ENODEV;
 
 		ch = dgnc_Board[board]->channels[channel];
drivers/staging/dgnc/dgnc_neo.c +3 -3
index 1268aa9..921e463 100644
@@ -407,7 +407,7 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
 	if (!brd || brd->magic != DGNC_BOARD_MAGIC)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];

@@ -537,7 +537,7 @@ static inline void neo_parse_lsr(struct dgnc_board *brd, uint port)
 	if (!brd || brd->magic != DGNC_BOARD_MAGIC)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];

@@ -1019,7 +1019,7 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
 			 */
 
 			/* Verify the port is in range. */
-			if (port > brd->nasync)
+			if (port >= brd->nasync)
 				continue;
 
 			ch = brd->channels[port];
drivers/staging/dgnc/dgnc_tty.c +1 -1
index 8179342..4507d4a 100644
@@ -1060,7 +1060,7 @@ static int dgnc_tty_open(struct tty_struct *tty, struct file *file)
 	spin_lock_irqsave(&brd->bd_lock, flags);
 
 	/* If opened device is greater than our number of ports, bail. */
-	if (PORT_NUM(minor) > brd->nasync) {
+	if (PORT_NUM(minor) >= brd->nasync) {
 		spin_unlock_irqrestore(&brd->bd_lock, flags);
 		return -ENXIO;
 	}