Skip to content

Commit 120fb87

Browse files
yuranpereiraDaniel Thompson
authored andcommitted
kdb: Replace the use of simple_strto with safer kstrto in kdb_main
The simple_str* family of functions perform no error checking in scenarios where the input value overflows the intended output variable. This results in these functions successfully returning even when the output does not match the input string. Or as it was mentioned [1], "...simple_strtol(), simple_strtoll(), simple_strtoul(), and simple_strtoull() functions explicitly ignore overflows, which may lead to unexpected results in callers." Hence, the use of those functions is discouraged. This patch replaces all uses of the simple_strto* series of functions with their safer kstrto* alternatives. Side effects of this patch: - Every string to long or long long conversion using kstrto* is now checked for failure. - kstrto* errors are handled with appropriate `KDB_BADINT` wherever applicable. - A good side effect is that we end up saving a few lines of code since unlike in simple_strto* functions, kstrto functions do not need an additional "end pointer" variable, and the return values of the latter can be directly checked in an "if" statement without the need to define additional `ret` or `err` variables. This, of course, results in cleaner, yet still easy to understand code. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull Signed-off-by: Yuran Pereira <[email protected]> [nir: addressed review comments by fixing styling, invalid conversion and a missing error return] Signed-off-by: Nir Lichtman <[email protected]> Reviewed-by: Douglas Anderson <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Daniel Thompson <[email protected]>
1 parent 8198375 commit 120fb87

File tree

1 file changed

+21
-48
lines changed

1 file changed

+21
-48
lines changed

kernel/debug/kdb/kdb_main.c

Lines changed: 21 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -306,8 +306,8 @@ static int kdbgetulenv(const char *match, unsigned long *value)
306306
return KDB_NOTENV;
307307
if (strlen(ep) == 0)
308308
return KDB_NOENVVALUE;
309-
310-
*value = simple_strtoul(ep, NULL, 0);
309+
if (kstrtoul(ep, 0, value))
310+
return KDB_BADINT;
311311

312312
return 0;
313313
}
@@ -402,42 +402,23 @@ static void kdb_printenv(void)
402402
*/
403403
int kdbgetularg(const char *arg, unsigned long *value)
404404
{
405-
char *endp;
406-
unsigned long val;
407-
408-
val = simple_strtoul(arg, &endp, 0);
409-
410-
if (endp == arg) {
411-
/*
412-
* Also try base 16, for us folks too lazy to type the
413-
* leading 0x...
414-
*/
415-
val = simple_strtoul(arg, &endp, 16);
416-
if (endp == arg)
405+
/*
406+
* If the first fails, also try base 16, for us
407+
* folks too lazy to type the leading 0x...
408+
*/
409+
if (kstrtoul(arg, 0, value)) {
410+
if (kstrtoul(arg, 16, value))
417411
return KDB_BADINT;
418412
}
419-
420-
*value = val;
421-
422413
return 0;
423414
}
424415

425416
int kdbgetu64arg(const char *arg, u64 *value)
426417
{
427-
char *endp;
428-
u64 val;
429-
430-
val = simple_strtoull(arg, &endp, 0);
431-
432-
if (endp == arg) {
433-
434-
val = simple_strtoull(arg, &endp, 16);
435-
if (endp == arg)
418+
if (kstrtou64(arg, 0, value)) {
419+
if (kstrtou64(arg, 16, value))
436420
return KDB_BADINT;
437421
}
438-
439-
*value = val;
440-
441422
return 0;
442423
}
443424

@@ -473,10 +454,10 @@ int kdb_set(int argc, const char **argv)
473454
*/
474455
if (strcmp(argv[1], "KDBDEBUG") == 0) {
475456
unsigned int debugflags;
476-
char *cp;
457+
int ret;
477458

478-
debugflags = simple_strtoul(argv[2], &cp, 0);
479-
if (cp == argv[2] || debugflags & ~KDB_DEBUG_FLAG_MASK) {
459+
ret = kstrtouint(argv[2], 0, &debugflags);
460+
if (ret || debugflags & ~KDB_DEBUG_FLAG_MASK) {
480461
kdb_printf("kdb: illegal debug flags '%s'\n",
481462
argv[2]);
482463
return 0;
@@ -1619,10 +1600,10 @@ static int kdb_md(int argc, const char **argv)
16191600
if (!argv[0][3])
16201601
valid = 1;
16211602
else if (argv[0][3] == 'c' && argv[0][4]) {
1622-
char *p;
1623-
repeat = simple_strtoul(argv[0] + 4, &p, 10);
1603+
if (kstrtouint(argv[0] + 4, 10, &repeat))
1604+
return KDB_BADINT;
16241605
mdcount = ((repeat * bytesperword) + 15) / 16;
1625-
valid = !*p;
1606+
valid = 1;
16261607
}
16271608
last_repeat = repeat;
16281609
} else if (strcmp(argv[0], "md") == 0)
@@ -2083,15 +2064,10 @@ static int kdb_dmesg(int argc, const char **argv)
20832064
if (argc > 2)
20842065
return KDB_ARGCOUNT;
20852066
if (argc) {
2086-
char *cp;
2087-
lines = simple_strtol(argv[1], &cp, 0);
2088-
if (*cp)
2067+
if (kstrtoint(argv[1], 0, &lines))
20892068
lines = 0;
2090-
if (argc > 1) {
2091-
adjust = simple_strtoul(argv[2], &cp, 0);
2092-
if (*cp || adjust < 0)
2093-
adjust = 0;
2094-
}
2069+
if (argc > 1 && (kstrtoint(argv[2], 0, &adjust) || adjust < 0))
2070+
adjust = 0;
20952071
}
20962072

20972073
/* disable LOGGING if set */
@@ -2428,23 +2404,20 @@ static int kdb_help(int argc, const char **argv)
24282404
static int kdb_kill(int argc, const char **argv)
24292405
{
24302406
long sig, pid;
2431-
char *endp;
24322407
struct task_struct *p;
24332408

24342409
if (argc != 2)
24352410
return KDB_ARGCOUNT;
24362411

2437-
sig = simple_strtol(argv[1], &endp, 0);
2438-
if (*endp)
2412+
if (kstrtol(argv[1], 0, &sig))
24392413
return KDB_BADINT;
24402414
if ((sig >= 0) || !valid_signal(-sig)) {
24412415
kdb_printf("Invalid signal parameter.<-signal>\n");
24422416
return 0;
24432417
}
24442418
sig = -sig;
24452419

2446-
pid = simple_strtol(argv[2], &endp, 0);
2447-
if (*endp)
2420+
if (kstrtol(argv[2], 0, &pid))
24482421
return KDB_BADINT;
24492422
if (pid <= 0) {
24502423
kdb_printf("Process ID must be large than 0.\n");

0 commit comments

Comments
 (0)