Skip to contentSkip to conversation

misc: kgdbts: silence array underflow warning

PATCH In review Dan Carpenter 2 participants 2 messages May 17, 2018

Subsystems (3)
Kernel janitors Char and misc drivers KGDB / KDB /debug_core
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 misc: kgdbts: silence array underflow warning

Smatch distrusts simple_strtol(). I don't know the code well enough to say if the distrust is justified here, but it seems harmless to silence the warning.

Signed-off-by Dan Carpenter <[email protected]>
drivers/misc/kgdbts.c +3 -1
index 6193270e7b3d..e0508acaedaa 100644
@@ -400,13 +400,15 @@ static void skip_back_repeat_test(char *arg)
 	int go_back = simple_strtol(arg, NULL, 10);
 
 	repeat_test--;
-	if (repeat_test <= 0) {
+	if (repeat_test <= 0 || go_back < 0) {
 		ts.idx++;
 	} else {
 		if (repeat_test % 100 = 0)
 			v1printk("kgdbts:RUN ... %d remaining\n", repeat_test);
 
 		ts.idx -= go_back;
+		if (ts.idx < 0)
+			ts.idx = 0;
 	}
 	fill_get_buf(ts.tst[ts.idx].get);
 }
#1 lore raw
Quoted 3 lines · Dan Carpenter

Smatch distrusts simple_strtol(). I don't know the code well enough to say if the distrust is justified here, but it seems harmless to silence the warning.

What warning does this fix? I'd prefer to have it in the description.

drivers/misc/kgdbts.c static void skip_back_repeat_test(char *arg) · line 400

Quoted 8 lines
Signed-off-by Dan Carpenter <[email protected]>
drivers/misc/kgdbts.c +0 -0
index 6193270e7b3d..e0508acaedaa 100644
@@ -400,13 +400,15 @@ static void skip_back_repeat_test(char *arg)
 	int go_back = simple_strtol(arg, NULL, 10);

If go_back is out of range then this is a serious error in the test plans found in the module. Something simple and clear such as BUG_ON(go_back <= 0 || go_back > ts.idx) is probably sufficient.

drivers/misc/kgdbts.c static void skip_back_repeat_test(char *arg) · line 403

Quoted 3 lines
drivers/misc/kgdbts.c +1 -1
 	repeat_test--;
-	if (repeat_test <= 0) {
+	if (repeat_test <= 0 || go_back < 0) {

The BUG_ON() will mess things up because whatever breakpoints the test is using are still enabled.

If you really want to recover cleanly then perhaps:

+ if (repeat_test <= 0 || WARN_ON(go_back < 0 || go_back > ts.idx)) {

drivers/misc/kgdbts.c static void skip_back_repeat_test(char *arg) · line 411

Quoted 8 lines
drivers/misc/kgdbts.c +2 -0
 		ts.idx++;
 	} else {
 		if (repeat_test % 100 = 0)
 			v1printk("kgdbts:RUN ... %d remaining\n", repeat_test);
 
 		ts.idx -= go_back;
+		if (ts.idx < 0)
+			ts.idx = 0;

Not sure about this. If we know the opcodes are bad then re-executing them doesn't seem like a good idea (hence covering it in the WARN_ON branch above).

Daniel.