From c090e8f2367f1c532bf30935104eccdada9d013d Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:34 +0200 Subject: stdio: Get rid of dead code, i.e. stdio_deregister() Nobody is using stdio_deregister(), remove for good. Note, even its parameters are not consistent with stdio_register(). So, if anyone want to introduce this again, better with some consistency. Signed-off-by: Andy Shevchenko --- common/stdio.c | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'common') diff --git a/common/stdio.c b/common/stdio.c index 2b883fddbea..acc65ada1b1 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -261,17 +261,6 @@ int stdio_deregister_dev(struct stdio_dev *dev, int force) return 0; } -int stdio_deregister(const char *devname, int force) -{ - struct stdio_dev *dev; - - dev = stdio_get_by_name(devname); - if (!dev) /* device not found */ - return -ENODEV; - - return stdio_deregister_dev(dev, force); -} - int stdio_init_tables(void) { #if defined(CONFIG_NEEDS_MANUAL_RELOC) -- cgit v1.3.1 From 99cb2b996bd649d98069a95941beaaade0a4447a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:35 +0200 Subject: stdio: Split out nulldev_register() and move it under #if It's possible that NULLDEV can be disabled while it makes leftovers, move entire device under #if. Signed-off-by: Andy Shevchenko --- common/stdio.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) (limited to 'common') diff --git a/common/stdio.c b/common/stdio.c index acc65ada1b1..61fc0873684 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -28,6 +28,7 @@ static struct stdio_dev devs; struct stdio_dev *stdio_devices[] = { NULL, NULL, NULL }; char *stdio_names[MAX_FILES] = { "stdin", "stdout", "stderr" }; +#if CONFIG_IS_ENABLED(SYS_DEVICE_NULLDEV) static void nulldev_putc(struct stdio_dev *dev, const char c) { /* nulldev is empty! */ @@ -44,6 +45,25 @@ static int nulldev_input(struct stdio_dev *dev) return 0; } +static void nulldev_register(void) +{ + struct stdio_dev dev; + + memset(&dev, '\0', sizeof(dev)); + + strcpy(dev.name, "nulldev"); + dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT; + dev.putc = nulldev_putc; + dev.puts = nulldev_puts; + dev.getc = nulldev_input; + dev.tstc = nulldev_input; + + stdio_register(&dev); +} +#else +static inline void nulldev_register(void) {} +#endif /* SYS_DEVICE_NULLDEV */ + static void stdio_serial_putc(struct stdio_dev *dev, const char c) { serial_putc(c); @@ -83,18 +103,7 @@ static void drv_system_init (void) dev.tstc = stdio_serial_tstc; stdio_register (&dev); - if (CONFIG_IS_ENABLED(SYS_DEVICE_NULLDEV)) { - memset(&dev, '\0', sizeof(dev)); - - strcpy(dev.name, "nulldev"); - dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT; - dev.putc = nulldev_putc; - dev.puts = nulldev_puts; - dev.getc = nulldev_input; - dev.tstc = nulldev_input; - - stdio_register(&dev); - } + nulldev_register(); } /************************************************************************** -- cgit v1.3.1 From d9b0ac90baf499d215462ed7afffbfd22a58340b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:36 +0200 Subject: stdio: Introduce a new helper stdio_file_to_flags() Let's deduplicate existing copies by splitting off to a new helper. Signed-off-by: Andy Shevchenko --- common/stdio.c | 13 +++++++++++++ include/stdio_dev.h | 2 ++ 2 files changed, 15 insertions(+) (limited to 'common') diff --git a/common/stdio.c b/common/stdio.c index 61fc0873684..d4acc5256c1 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -28,6 +28,19 @@ static struct stdio_dev devs; struct stdio_dev *stdio_devices[] = { NULL, NULL, NULL }; char *stdio_names[MAX_FILES] = { "stdin", "stdout", "stderr" }; +int stdio_file_to_flags(const int file) +{ + switch (file) { + case stdin: + return DEV_FLAGS_INPUT; + case stdout: + case stderr: + return DEV_FLAGS_OUTPUT; + default: + return -EINVAL; + } +} + #if CONFIG_IS_ENABLED(SYS_DEVICE_NULLDEV) static void nulldev_putc(struct stdio_dev *dev, const char c) { diff --git a/include/stdio_dev.h b/include/stdio_dev.h index 109a68d0640..8fb9a12dd87 100644 --- a/include/stdio_dev.h +++ b/include/stdio_dev.h @@ -18,6 +18,8 @@ #define DEV_FLAGS_OUTPUT 0x00000002 /* Device can be used as output console */ #define DEV_FLAGS_DM 0x00000004 /* Device priv is a struct udevice * */ +int stdio_file_to_flags(const int file); + /* Device information */ struct stdio_dev { int flags; /* Device flags: input/output/system */ -- cgit v1.3.1 From 7b9ca3f89c340f3a195e17ccdd7d512e0884e555 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:37 +0200 Subject: console: Switch to use stdio_file_to_flags() Deduplicate code by replacing with stdio_file_to_flags() helper. Signed-off-by: Andy Shevchenko --- common/console.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'common') diff --git a/common/console.c b/common/console.c index 567273a0ce8..b1afb650ed3 100644 --- a/common/console.c +++ b/common/console.c @@ -855,17 +855,9 @@ int console_assign(int file, const char *devname) struct stdio_dev *dev; /* Check for valid file */ - switch (file) { - case stdin: - flag = DEV_FLAGS_INPUT; - break; - case stdout: - case stderr: - flag = DEV_FLAGS_OUTPUT; - break; - default: - return -1; - } + flag = stdio_file_to_flags(file); + if (flag < 0) + return flag; /* Check for valid device name */ -- cgit v1.3.1 From 20a7d351486f9d5afde55ffd9d8331e765add42d Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:38 +0200 Subject: console: Set console device counter in console_devices_set() console_devices_set() missed the console device counter to be set correctly. Fixes: 45375adc9799 ("console: add function console_devices_set") Cc: Patrick Delaunay Signed-off-by: Andy Shevchenko --- common/console.c | 1 + 1 file changed, 1 insertion(+) (limited to 'common') diff --git a/common/console.c b/common/console.c index b1afb650ed3..9064a6c2303 100644 --- a/common/console.c +++ b/common/console.c @@ -236,6 +236,7 @@ int cd_count[MAX_FILES]; static void __maybe_unused console_devices_set(int file, struct stdio_dev *dev) { console_devices[file][0] = dev; + cd_count[file] = 1; } /** -- cgit v1.3.1 From 09d8f07762747ca6cd530289ecdda21b1c92226b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:39 +0200 Subject: console: Set file and devices at one go Logical continuation of the change that brought console_devices_set() is to unify console_setfile() with it and replace in the callers. Signed-off-by: Andy Shevchenko --- common/console.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'common') diff --git a/common/console.c b/common/console.c index 9064a6c2303..7b6861f2147 100644 --- a/common/console.c +++ b/common/console.c @@ -233,7 +233,7 @@ static struct stdio_dev *tstcdev; struct stdio_dev **console_devices[MAX_FILES]; int cd_count[MAX_FILES]; -static void __maybe_unused console_devices_set(int file, struct stdio_dev *dev) +static void console_devices_set(int file, struct stdio_dev *dev) { console_devices[file][0] = dev; cd_count[file] = 1; @@ -370,7 +370,7 @@ static inline void console_doenv(int file, struct stdio_dev *dev) #endif #else -static void __maybe_unused console_devices_set(int file, struct stdio_dev *dev) +static void console_devices_set(int file, struct stdio_dev *dev) { } @@ -418,6 +418,12 @@ static inline void console_doenv(int file, struct stdio_dev *dev) #endif #endif /* CONIFIG_IS_ENABLED(CONSOLE_MUX) */ +static void __maybe_unused console_setfile_and_devices(int file, struct stdio_dev *dev) +{ + console_setfile(file, dev); + console_devices_set(file, dev); +} + int console_start(int file, struct stdio_dev *sdev) { int error; @@ -1072,17 +1078,13 @@ int console_init_r(void) /* Initializes output console first */ if (outputdev != NULL) { - console_setfile(stdout, outputdev); - console_setfile(stderr, outputdev); - console_devices_set(stdout, outputdev); - console_devices_set(stderr, outputdev); + console_setfile_and_devices(stdout, outputdev); + console_setfile_and_devices(stderr, outputdev); } /* Initializes input console */ - if (inputdev != NULL) { - console_setfile(stdin, inputdev); - console_devices_set(stdin, inputdev); - } + if (inputdev != NULL) + console_setfile_and_devices(stdin, inputdev); if (!IS_ENABLED(CONFIG_SYS_CONSOLE_INFO_QUIET)) stdio_print_current_devices(); -- cgit v1.3.1 From 658d6c583663ae4f37cb6a5e104b2bf864fbc497 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:40 +0200 Subject: IOMUX: Switch to use stdio_file_to_flags() Deduplicate code by replacing with stdio_file_to_flags() helper. Signed-off-by: Andy Shevchenko --- common/iomux.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'common') diff --git a/common/iomux.c b/common/iomux.c index 15bf5338855..5d027561bb6 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -75,15 +75,8 @@ int iomux_doenv(const int console, const char *arg) return 1; } - switch (console) { - case stdin: - io_flag = DEV_FLAGS_INPUT; - break; - case stdout: - case stderr: - io_flag = DEV_FLAGS_OUTPUT; - break; - default: + io_flag = stdio_file_to_flags(console); + if (io_flag < 0) { free(start); free(console_args); free(cons_set); -- cgit v1.3.1 From b672c1619bb9615aff3ebbe15c20083fd0f58f9b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:41 +0200 Subject: IOMUX: Split out iomux_match_device() helper Deduplicate the code used in a few places by splitting out a common helper. Signed-off-by: Andy Shevchenko --- common/console.c | 7 +++---- common/iomux.c | 27 ++++++++++++++------------- include/iomux.h | 1 + 3 files changed, 18 insertions(+), 17 deletions(-) (limited to 'common') diff --git a/common/console.c b/common/console.c index 7b6861f2147..523fb45a99e 100644 --- a/common/console.c +++ b/common/console.c @@ -252,15 +252,14 @@ static void console_devices_set(int file, struct stdio_dev *dev) */ static bool console_needs_start_stop(int file, struct stdio_dev *sdev) { - int i, j; + int i; for (i = 0; i < ARRAY_SIZE(cd_count); i++) { if (i == file) continue; - for (j = 0; j < cd_count[i]; j++) - if (console_devices[i][j] == sdev) - return false; + if (iomux_match_device(console_devices[i], cd_count[i], sdev) >= 0) + return false; } return true; } diff --git a/common/iomux.c b/common/iomux.c index 5d027561bb6..a8be1ac7d8a 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -22,11 +22,21 @@ void iomux_printdevs(const int console) printf("\n"); } +int iomux_match_device(struct stdio_dev **set, const int n, struct stdio_dev *sdev) +{ + int i; + + for (i = 0; i < n; i++) + if (sdev == set[i]) + return i; + return -ENOENT; +} + /* This tries to preserve the old list if an error occurs. */ int iomux_doenv(const int console, const char *arg) { char *console_args, *temp, **start; - int i, j, k, io_flag, cs_idx, repeat; + int i, j, io_flag, cs_idx, repeat; struct stdio_dev **cons_set, **old_set; struct stdio_dev *dev; @@ -96,14 +106,8 @@ int iomux_doenv(const int console, const char *arg) /* * Prevent multiple entries for a device. */ - repeat = 0; - for (k = 0; k < cs_idx; k++) { - if (dev == cons_set[k]) { - repeat++; - break; - } - } - if (repeat) + repeat = iomux_match_device(cons_set, cs_idx, dev); + if (repeat >= 0) continue; /* * Try assigning the specified device. @@ -129,10 +133,7 @@ int iomux_doenv(const int console, const char *arg) /* Stop dropped consoles */ for (i = 0; i < repeat; i++) { - for (j = 0; j < cs_idx; j++) { - if (old_set[i] == cons_set[j]) - break; - } + j = iomux_match_device(cons_set, cs_idx, old_set[i]); if (j == cs_idx) console_stop(console, old_set[i]); } diff --git a/include/iomux.h b/include/iomux.h index da7ff697d21..9c2d5796066 100644 --- a/include/iomux.h +++ b/include/iomux.h @@ -24,6 +24,7 @@ extern struct stdio_dev **console_devices[MAX_FILES]; */ extern int cd_count[MAX_FILES]; +int iomux_match_device(struct stdio_dev **, const int, struct stdio_dev *); int iomux_doenv(const int, const char *); void iomux_printdevs(const int); -- cgit v1.3.1 From 400797cad36850797307be3c56d2d5bc16aa02bb Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:42 +0200 Subject: IOMUX: Split out for_each_console_dev() helper macro It is not only less lines of code, but also better readability when new macro is being in use. Introduce for_each_console_dev() helper macro and convert current users to it. Signed-off-by: Andy Shevchenko --- common/console.c | 15 +++++---------- common/iomux.c | 4 +--- include/iomux.h | 5 +++++ 3 files changed, 11 insertions(+), 13 deletions(-) (limited to 'common') diff --git a/common/console.c b/common/console.c index 523fb45a99e..561cdf36a74 100644 --- a/common/console.c +++ b/common/console.c @@ -293,8 +293,7 @@ static int console_tstc(int file) int prev; prev = disable_ctrlc(1); - for (i = 0; i < cd_count[file]; i++) { - dev = console_devices[file][i]; + for_each_console_dev(i, file, dev) { if (dev->tstc != NULL) { ret = dev->tstc(dev); if (ret > 0) { @@ -314,8 +313,7 @@ static void console_putc(int file, const char c) int i; struct stdio_dev *dev; - for (i = 0; i < cd_count[file]; i++) { - dev = console_devices[file][i]; + for_each_console_dev(i, file, dev) { if (dev->putc != NULL) dev->putc(dev, c); } @@ -334,11 +332,9 @@ static void console_puts_select(int file, bool serial_only, const char *s) int i; struct stdio_dev *dev; - for (i = 0; i < cd_count[file]; i++) { - bool is_serial; + for_each_console_dev(i, file, dev) { + bool is_serial = console_dev_is_serial(dev); - dev = console_devices[file][i]; - is_serial = console_dev_is_serial(dev); if (dev->puts && serial_only == is_serial) dev->puts(dev, s); } @@ -354,8 +350,7 @@ static void console_puts(int file, const char *s) int i; struct stdio_dev *dev; - for (i = 0; i < cd_count[file]; i++) { - dev = console_devices[file][i]; + for_each_console_dev(i, file, dev) { if (dev->puts != NULL) dev->puts(dev, s); } diff --git a/common/iomux.c b/common/iomux.c index a8be1ac7d8a..5290b13b668 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -15,10 +15,8 @@ void iomux_printdevs(const int console) int i; struct stdio_dev *dev; - for (i = 0; i < cd_count[console]; i++) { - dev = console_devices[console][i]; + for_each_console_dev(i, console, dev) printf("%s ", dev->name); - } printf("\n"); } diff --git a/include/iomux.h b/include/iomux.h index 9c2d5796066..bd4a143b1e6 100644 --- a/include/iomux.h +++ b/include/iomux.h @@ -24,6 +24,11 @@ extern struct stdio_dev **console_devices[MAX_FILES]; */ extern int cd_count[MAX_FILES]; +#define for_each_console_dev(i, file, dev) \ + for (i = 0, dev = console_devices[file][i]; \ + i < cd_count[file]; \ + i++, dev = console_devices[file][i]) + int iomux_match_device(struct stdio_dev **, const int, struct stdio_dev *); int iomux_doenv(const int, const char *); void iomux_printdevs(const int); -- cgit v1.3.1 From 694cd5618c2ee263c025462e780354f28313b7a3 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:43 +0200 Subject: IOMUX: Introduce iomux_replace_device() Some console devices may appear or disappear at run time. In order to support such a hotplug mechanism introduce a new iomux_replace_device() helper to update the list of devices without altering environment. Signed-off-by: Andy Shevchenko --- common/iomux.c | 33 +++++++++++++++++++++++++++++++++ include/iomux.h | 1 + 2 files changed, 34 insertions(+) (limited to 'common') diff --git a/common/iomux.c b/common/iomux.c index 5290b13b668..b9088aa3b58 100644 --- a/common/iomux.c +++ b/common/iomux.c @@ -139,4 +139,37 @@ int iomux_doenv(const int console, const char *arg) free(old_set); return 0; } + +int iomux_replace_device(const int console, const char *old, const char *new) +{ + struct stdio_dev *dev; + char *arg = NULL; /* Initial empty list */ + int size = 1; /* For NUL terminator */ + int i, ret; + + for_each_console_dev(i, console, dev) { + const char *name = strcmp(dev->name, old) ? dev->name : new; + char *tmp; + + /* Append name with a ',' (comma) separator */ + tmp = realloc(arg, size + strlen(name) + 1); + if (!tmp) { + free(arg); + return -ENOMEM; + } + + strcat(tmp, ","); + strcat(tmp, name); + + arg = tmp; + size = strlen(tmp) + 1; + } + + ret = iomux_doenv(console, arg); + if (ret) + ret = -EINVAL; + + free(arg); + return ret; +} #endif /* CONSOLE_MUX */ diff --git a/include/iomux.h b/include/iomux.h index bd4a143b1e6..37f5f6dee69 100644 --- a/include/iomux.h +++ b/include/iomux.h @@ -31,6 +31,7 @@ extern int cd_count[MAX_FILES]; int iomux_match_device(struct stdio_dev **, const int, struct stdio_dev *); int iomux_doenv(const int, const char *); +int iomux_replace_device(const int, const char *, const char *); void iomux_printdevs(const int); #endif /* _IO_MUX_H */ -- cgit v1.3.1 From eb5fd9e46c11ea41430d9c5bcc81d4583424216e Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 11 Feb 2021 17:09:44 +0200 Subject: usb: kbd: destroy device after console is stopped In case of IOMUX enabled it assumes that console devices in the list are available to get them stopped properly via ->stop() callback. However, the USB keyboard driver violates this assumption and tries to play tricks so the device get destroyed while being listed as an active console. Swap the order of device deregistration and IOMUX update along with converting to use iomux_replace_device() jelper to avoid the use-after-free. Fixes: 3cbcb2892809 ("usb: Fix usb_kbd_deregister when console-muxing is used") Fixes: 8a8348703081 ("dm: usb: Add a remove() method for USB keyboards") Reported-by: Nicolas Saenz Julienne Signed-off-by: Andy Shevchenko --- common/usb_kbd.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'common') diff --git a/common/usb_kbd.c b/common/usb_kbd.c index b316807844b..60c6027e048 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -617,12 +617,12 @@ int usb_kbd_deregister(int force) if (dev) { usb_kbd_dev = (struct usb_device *)dev->priv; data = usb_kbd_dev->privptr; - if (stdio_deregister_dev(dev, force) != 0) - return 1; #if CONFIG_IS_ENABLED(CONSOLE_MUX) - if (iomux_doenv(stdin, env_get("stdin")) != 0) + if (iomux_replace_device(stdin, DEVNAME, force ? "nulldev" : "")) return 1; #endif + if (stdio_deregister_dev(dev, force) != 0) + return 1; #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE destroy_int_queue(usb_kbd_dev, data->intq); #endif @@ -660,16 +660,16 @@ static int usb_kbd_remove(struct udevice *dev) goto err; } data = udev->privptr; - if (stdio_deregister_dev(sdev, true)) { - ret = -EPERM; - goto err; - } #if CONFIG_IS_ENABLED(CONSOLE_MUX) - if (iomux_doenv(stdin, env_get("stdin"))) { + if (iomux_replace_device(stdin, DEVNAME, "nulldev")) { ret = -ENOLINK; goto err; } #endif + if (stdio_deregister_dev(sdev, true)) { + ret = -EPERM; + goto err; + } #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE destroy_int_queue(udev, data->intq); #endif -- cgit v1.3.1