Skip to contentSkip to conversation

serial: jsm: some off by one bugs

PATCH Superseded Dan Carpenter 2 participants 5 messages Mar 12, 2015 – Mar 17, 2015

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 serial: jsm: some off by one bugs

"brd->nasync" amd "brd->maxports" are the same. They hold the number of filled out channels in the brd->channels[] array. These tests should be ">=" instead of ">" so that we don't read one element past the end.

drivers/tty/serial/jsm/jsm_cls.c +1 -1
index bfb0681..4eb12a9 100644
@@ -570,7 +570,7 @@ static inline void cls_parse_isr(struct jsm_board *brd, uint port)
 	 * verified in the interrupt routine.
 	 */
 
-	if (port > brd->nasync)
+	if (port >= brd->nasync)
 		return;
 
 	ch = brd->channels[port];
drivers/tty/serial/jsm/jsm_neo.c +2 -2
index 7291c21..f413ef0 100644
@@ -840,7 +840,7 @@ static inline void neo_parse_lsr(struct jsm_board *brd, u32 port)
 	if (!brd)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];
@@ -1180,7 +1180,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];
#1 lore raw

drivers/tty/serial/jsm/jsm_neo.c

Quoted 23 lines · Dan Carpenter

"brd->nasync" amd "brd->maxports" are the same. They hold the number of filled out channels in the brd->channels[] array. These tests should be ">=" instead of ">" so that we don't read one element past the end.

drivers/tty/serial/jsm/jsm_cls.c +1 -1
index bfb0681..4eb12a9 100644
@@ -570,7 +570,7 @@ static inline void cls_parse_isr(struct jsm_board *brd, uint port)
 	 * verified in the interrupt routine.
 	 */
 
-	if (port > brd->nasync)
+	if (port >= brd->nasync)
 		return;
 
 	ch = brd->channels[port];
drivers/tty/serial/jsm/jsm_neo.c +0 -0
index 7291c21..f413ef0 100644

Hi, Dan.

It looks like you missed the fix for neo_parse_isr.

Would you send a v2 fixing that as well?

Thanks.
Cascardo.

Quoted 19 lines
drivers/tty/serial/jsm/jsm_neo.c +2 -2
@@ -840,7 +840,7 @@ static inline void neo_parse_lsr(struct jsm_board *brd, u32 port)
 	if (!brd)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];
@@ -1180,7 +1180,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];
#2 lore raw
Quoted 27 lines · On Mon, Mar 16, 2015 at 02:47:43PM -0300, [email protected] wrote:

drivers/tty/serial/jsm/jsm_neo.c

Quoted 23 lines · Dan Carpenter

"brd->nasync" amd "brd->maxports" are the same. They hold the number of filled out channels in the brd->channels[] array. These tests should be ">=" instead of ">" so that we don't read one element past the end.

drivers/tty/serial/jsm/jsm_cls.c +1 -1
index bfb0681..4eb12a9 100644
@@ -570,7 +570,7 @@ static inline void cls_parse_isr(struct jsm_board *brd, uint port)
 	 * verified in the interrupt routine.
 	 */
 
-	if (port > brd->nasync)
+	if (port >= brd->nasync)
 		return;
 
 	ch = brd->channels[port];
drivers/tty/serial/jsm/jsm_neo.c +0 -0
index 7291c21..f413ef0 100644

Hi, Dan.

Quoted 3 lines

It looks like you missed the fix for neo_parse_isr.

Would you send a v2 fixing that as well?

Yes. Thanks for catching this.

regards,
dan carpenter

#3 lore raw

PATCH v2 2/2 serial: jsm: some off by one bugs

In reply to [email protected]

"brd->nasync" amd "brd->maxports" are the same. They hold the number of filled out channels in the brd->channels[] array. These tests should be ">=" instead of ">" so that we don't read one element past the end.

---
v2: Thadeu Cascardo noticed another off by one that I missed.

drivers/tty/serial/jsm/jsm_cls.c +1 -1
index bfb0681..4eb12a9 100644
@@ -570,7 +570,7 @@ static inline void cls_parse_isr(struct jsm_board *brd, uint port)
 	 * verified in the interrupt routine.
 	 */
 
-	if (port > brd->nasync)
+	if (port >= brd->nasync)
 		return;
 
 	ch = brd->channels[port];
drivers/tty/serial/jsm/jsm_neo.c +3 -3
index 7291c21..932b2ac 100644
@@ -724,7 +724,7 @@ static inline void neo_parse_isr(struct jsm_board *brd, u32 port)
 	if (!brd)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];
@@ -840,7 +840,7 @@ static inline void neo_parse_lsr(struct jsm_board *brd, u32 port)
 	if (!brd)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];
@@ -1180,7 +1180,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];
Acked-by on v2 2/2
#4 lore raw
Quoted 8 lines · Dan Carpenter

"brd->nasync" amd "brd->maxports" are the same. They hold the number of filled out channels in the brd->channels[] array. These tests should be ">=" instead of ">" so that we don't read one element past the end.

---
v2: Thadeu Cascardo noticed another off by one that I missed.

Quoted 45 lines
drivers/tty/serial/jsm/jsm_cls.c +1 -1
index bfb0681..4eb12a9 100644
@@ -570,7 +570,7 @@ static inline void cls_parse_isr(struct jsm_board *brd, uint port)
 	 * verified in the interrupt routine.
 	 */
 
-	if (port > brd->nasync)
+	if (port >= brd->nasync)
 		return;
 
 	ch = brd->channels[port];
drivers/tty/serial/jsm/jsm_neo.c +3 -3
index 7291c21..932b2ac 100644
@@ -724,7 +724,7 @@ static inline void neo_parse_isr(struct jsm_board *brd, u32 port)
 	if (!brd)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];
@@ -840,7 +840,7 @@ static inline void neo_parse_lsr(struct jsm_board *brd, u32 port)
 	if (!brd)
 		return;
 
-	if (port > brd->maxports)
+	if (port >= brd->maxports)
 		return;
 
 	ch = brd->channels[port];
@@ -1180,7 +1180,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];