diff options
| -rw-r--r-- | NEWS | 5 | ||||
| -rw-r--r-- | src/stty.c | 358 | ||||
| -rwxr-xr-x | tests/misc/stty.sh | 8 |
3 files changed, 209 insertions, 162 deletions
@@ -13,6 +13,11 @@ GNU coreutils NEWS -*- outline -*- depending on the size of the first file processed. [bug introduced in coreutils-8.24] +** Improvements + + stty now validates arguments before interacting with the device, + ensuring there are no side effects to specifying an invalid option. + * Noteworthy changes in release 8.26 (2016-11-30) [stable] diff --git a/src/stty.c b/src/stty.c index f8aefff62..4f911a006 100644 --- a/src/stty.c +++ b/src/stty.c @@ -1077,143 +1077,21 @@ settings, CHAR is taken literally, or coded as in ^c, 0x37, 0177 or\n\ exit (status); } -int -main (int argc, char **argv) -{ - /* Initialize to all zeroes so there is no risk memcmp will report a - spurious difference in an uninitialized portion of the structure. */ - static struct termios mode; - - enum output_type output_type; - int optc; - int argi = 0; - int opti = 1; - bool require_set_attr; - bool speed_was_set _GL_UNUSED; - bool verbose_output; - bool recoverable_output; - int k; - bool noargs = true; - char *file_name = NULL; - const char *device_name; - - initialize_main (&argc, &argv); - set_program_name (argv[0]); - setlocale (LC_ALL, ""); - bindtextdomain (PACKAGE, LOCALEDIR); - textdomain (PACKAGE); - - atexit (close_stdout); - - output_type = changed; - verbose_output = false; - recoverable_output = false; - - /* Don't print error messages for unrecognized options. */ - opterr = 0; - - /* If any new options are ever added to stty, the short options MUST - NOT allow any ambiguity with the stty settings. For example, the - stty setting "-gagFork" would not be feasible, since it will be - parsed as "-g -a -g -F ork". If you change anything about how - stty parses options, be sure it still works with combinations of - short and long options, --, POSIXLY_CORRECT, etc. */ - - while ((optc = getopt_long (argc - argi, argv + argi, "-agF:", - longopts, NULL)) - != -1) - { - switch (optc) - { - case 'a': - verbose_output = true; - output_type = all; - break; - - case 'g': - recoverable_output = true; - output_type = recoverable; - break; - - case 'F': - if (file_name) - die (EXIT_FAILURE, 0, _("only one device may be specified")); - file_name = optarg; - break; - - case_GETOPT_HELP_CHAR; - - case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); - default: - /* Consider "drain" as an option rather than a setting, - to support: alias stty='stty -drain' etc. */ - if (! STREQ (argv[argi + opti], "-drain") - && ! STREQ (argv[argi + opti], "drain")) - noargs = false; +/* Apply specified settings to MODE, and update + SPEED_WAS_SET and REQUIRE_SET_ATTR as required. + If CHECKING is true, this function doesn't interact + with a device, and only validates specified settings. */ - /* Skip the argument containing this unrecognized option; - the 2nd pass will analyze it. */ - argi += opti; - - /* Restart getopt_long from the first unskipped argument. */ - opti = 1; - optind = 0; - - break; - } - - /* Clear fully-parsed arguments, so they don't confuse the 2nd pass. */ - while (opti < optind) - argv[argi + opti++] = NULL; - } - - /* Specifying both -a and -g gets an error. */ - if (verbose_output && recoverable_output) - die (EXIT_FAILURE, 0, - _("the options for verbose and stty-readable output styles are\n" - "mutually exclusive")); - - /* Specifying any other arguments with -a or -g gets an error. */ - if (!noargs && (verbose_output || recoverable_output)) - die (EXIT_FAILURE, 0, - _("when specifying an output style, modes may not be set")); - - /* FIXME: it'd be better not to open the file until we've verified - that all arguments are valid. Otherwise, we could end up doing - only some of the requested operations and then failing, probably - leaving things in an undesirable state. */ - - if (file_name) - { - int fdflags; - device_name = file_name; - if (fd_reopen (STDIN_FILENO, device_name, O_RDONLY | O_NONBLOCK, 0) < 0) - die (EXIT_FAILURE, errno, "%s", quotef (device_name)); - if ((fdflags = fcntl (STDIN_FILENO, F_GETFL)) == -1 - || fcntl (STDIN_FILENO, F_SETFL, fdflags & ~O_NONBLOCK) < 0) - die (EXIT_FAILURE, errno, _("%s: couldn't reset non-blocking mode"), - quotef (device_name)); - } - else - device_name = _("standard input"); - - if (tcgetattr (STDIN_FILENO, &mode)) - die (EXIT_FAILURE, errno, "%s", quotef (device_name)); - - if (verbose_output || recoverable_output || noargs) - { - max_col = screen_columns (); - current_col = 0; - display_settings (output_type, &mode, device_name); - return EXIT_SUCCESS; - } - - speed_was_set = false; - require_set_attr = false; - for (k = 1; k < argc; k++) +static void +apply_settings (bool checking, const char *device_name, + char * const *settings, int n_settings, + struct termios *mode, bool *speed_was_set, + bool *require_set_attr) +{ + for (int k = 1; k < n_settings; k++) { - char const *arg = argv[k]; + char const *arg = settings[k]; bool match_found = false; bool not_set_attr = false; bool reversed = false; @@ -1238,8 +1116,8 @@ main (int argc, char **argv) { if ((mode_info[i].flags & NO_SETATTR) == 0) { - match_found = set_mode (&mode_info[i], reversed, &mode); - require_set_attr = true; + match_found = set_mode (&mode_info[i], reversed, mode); + *require_set_attr = true; } else match_found = not_set_attr = true; @@ -1257,15 +1135,15 @@ main (int argc, char **argv) { if (STREQ (arg, control_info[i].name)) { - if (k == argc - 1) + if (k == n_settings - 1) { error (0, 0, _("missing argument to %s"), quote (arg)); usage (EXIT_FAILURE); } match_found = true; ++k; - set_control_char (&control_info[i], argv[k], &mode); - require_set_attr = true; + set_control_char (&control_info[i], settings[k], mode); + *require_set_attr = true; break; } } @@ -1274,27 +1152,31 @@ main (int argc, char **argv) { if (STREQ (arg, "ispeed")) { - if (k == argc - 1) + if (k == n_settings - 1) { error (0, 0, _("missing argument to %s"), quote (arg)); usage (EXIT_FAILURE); } ++k; - set_speed (input_speed, argv[k], &mode); - speed_was_set = true; - require_set_attr = true; + if (checking) + continue; + set_speed (input_speed, settings[k], mode); + *speed_was_set = true; + *require_set_attr = true; } else if (STREQ (arg, "ospeed")) { - if (k == argc - 1) + if (k == n_settings - 1) { error (0, 0, _("missing argument to %s"), quote (arg)); usage (EXIT_FAILURE); } ++k; - set_speed (output_speed, argv[k], &mode); - speed_was_set = true; - require_set_attr = true; + if (checking) + continue; + set_speed (output_speed, settings[k], mode); + *speed_was_set = true; + *require_set_attr = true; } #ifdef TIOCEXT /* This is the BSD interface to "extproc". @@ -1303,6 +1185,9 @@ main (int argc, char **argv) { int val = ! reversed; + if (checking) + continue; + if (ioctl (STDIN_FILENO, TIOCEXT, &val) != 0) { die (EXIT_FAILURE, errno, _("%s: error setting %s"), @@ -1313,29 +1198,35 @@ main (int argc, char **argv) #ifdef TIOCGWINSZ else if (STREQ (arg, "rows")) { - if (k == argc - 1) + if (k == n_settings - 1) { error (0, 0, _("missing argument to %s"), quote (arg)); usage (EXIT_FAILURE); } ++k; - set_window_size (integer_arg (argv[k], INT_MAX), -1, + if (checking) + continue; + set_window_size (integer_arg (settings[k], INT_MAX), -1, device_name); } else if (STREQ (arg, "cols") || STREQ (arg, "columns")) { - if (k == argc - 1) + if (k == n_settings - 1) { error (0, 0, _("missing argument to %s"), quote (arg)); usage (EXIT_FAILURE); } ++k; - set_window_size (-1, integer_arg (argv[k], INT_MAX), + if (checking) + continue; + set_window_size (-1, integer_arg (settings[k], INT_MAX), device_name); } else if (STREQ (arg, "size")) { + if (checking) + continue; max_col = screen_columns (); current_col = 0; display_window_size (false, device_name); @@ -1345,40 +1236,183 @@ main (int argc, char **argv) else if (STREQ (arg, "line")) { unsigned long int value; - if (k == argc - 1) + if (k == n_settings - 1) { error (0, 0, _("missing argument to %s"), quote (arg)); usage (EXIT_FAILURE); } ++k; - mode.c_line = value = integer_arg (argv[k], ULONG_MAX); - if (mode.c_line != value) - error (0, 0, _("invalid line discipline %s"), quote (argv[k])); - require_set_attr = true; + mode->c_line = value = integer_arg (settings[k], ULONG_MAX); + if (mode->c_line != value) + error (0, 0, _("invalid line discipline %s"), + quote (settings[k])); + *require_set_attr = true; } #endif else if (STREQ (arg, "speed")) { + if (checking) + continue; max_col = screen_columns (); - display_speed (&mode, false); + display_speed (mode, false); } else if (string_to_baud (arg) != (speed_t) -1) { - set_speed (both_speeds, arg, &mode); - speed_was_set = true; - require_set_attr = true; + if (checking) + continue; + set_speed (both_speeds, arg, mode); + *speed_was_set = true; + *require_set_attr = true; } else { - if (! recover_mode (arg, &mode)) + if (! recover_mode (arg, mode)) { error (0, 0, _("invalid argument %s"), quote (arg)); usage (EXIT_FAILURE); } - require_set_attr = true; + *require_set_attr = true; } } } +} + +int +main (int argc, char **argv) +{ + /* Initialize to all zeroes so there is no risk memcmp will report a + spurious difference in an uninitialized portion of the structure. */ + static struct termios mode; + + enum output_type output_type; + int optc; + int argi = 0; + int opti = 1; + bool require_set_attr; + bool speed_was_set _GL_UNUSED; + bool verbose_output; + bool recoverable_output; + bool noargs = true; + char *file_name = NULL; + const char *device_name; + + initialize_main (&argc, &argv); + set_program_name (argv[0]); + setlocale (LC_ALL, ""); + bindtextdomain (PACKAGE, LOCALEDIR); + textdomain (PACKAGE); + + atexit (close_stdout); + + output_type = changed; + verbose_output = false; + recoverable_output = false; + + /* Don't print error messages for unrecognized options. */ + opterr = 0; + + /* If any new options are ever added to stty, the short options MUST + NOT allow any ambiguity with the stty settings. For example, the + stty setting "-gagFork" would not be feasible, since it will be + parsed as "-g -a -g -F ork". If you change anything about how + stty parses options, be sure it still works with combinations of + short and long options, --, POSIXLY_CORRECT, etc. */ + + while ((optc = getopt_long (argc - argi, argv + argi, "-agF:", + longopts, NULL)) + != -1) + { + switch (optc) + { + case 'a': + verbose_output = true; + output_type = all; + break; + + case 'g': + recoverable_output = true; + output_type = recoverable; + break; + + case 'F': + if (file_name) + die (EXIT_FAILURE, 0, _("only one device may be specified")); + file_name = optarg; + break; + + case_GETOPT_HELP_CHAR; + + case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); + + default: + /* Consider "drain" as an option rather than a setting, + to support: alias stty='stty -drain' etc. */ + if (! STREQ (argv[argi + opti], "-drain") + && ! STREQ (argv[argi + opti], "drain")) + noargs = false; + + /* Skip the argument containing this unrecognized option; + the 2nd pass will analyze it. */ + argi += opti; + + /* Restart getopt_long from the first unskipped argument. */ + opti = 1; + optind = 0; + + break; + } + + /* Clear fully-parsed arguments, so they don't confuse the 2nd pass. */ + while (opti < optind) + argv[argi + opti++] = NULL; + } + + /* Specifying both -a and -g gets an error. */ + if (verbose_output && recoverable_output) + die (EXIT_FAILURE, 0, + _("the options for verbose and stty-readable output styles are\n" + "mutually exclusive")); + + /* Specifying any other arguments with -a or -g gets an error. */ + if (!noargs && (verbose_output || recoverable_output)) + die (EXIT_FAILURE, 0, + _("when specifying an output style, modes may not be set")); + + device_name = file_name ? file_name : _("standard input"); + + if (!noargs && !verbose_output && !recoverable_output) + { + static struct termios check_mode; + apply_settings (/* checking= */ true, device_name, argv, argc, + &check_mode, &speed_was_set, &require_set_attr); + } + + if (file_name) + { + int fdflags; + if (fd_reopen (STDIN_FILENO, device_name, O_RDONLY | O_NONBLOCK, 0) < 0) + die (EXIT_FAILURE, errno, "%s", quotef (device_name)); + if ((fdflags = fcntl (STDIN_FILENO, F_GETFL)) == -1 + || fcntl (STDIN_FILENO, F_SETFL, fdflags & ~O_NONBLOCK) < 0) + die (EXIT_FAILURE, errno, _("%s: couldn't reset non-blocking mode"), + quotef (device_name)); + } + + if (tcgetattr (STDIN_FILENO, &mode)) + die (EXIT_FAILURE, errno, "%s", quotef (device_name)); + + if (verbose_output || recoverable_output || noargs) + { + max_col = screen_columns (); + current_col = 0; + display_settings (output_type, &mode, device_name); + return EXIT_SUCCESS; + } + + speed_was_set = false; + require_set_attr = false; + apply_settings (/* checking= */ false, device_name, argv, argc, + &mode, &speed_was_set, &require_set_attr); if (require_set_attr) { diff --git a/tests/misc/stty.sh b/tests/misc/stty.sh index f6200e715..e549adb65 100755 --- a/tests/misc/stty.sh +++ b/tests/misc/stty.sh @@ -22,6 +22,7 @@ print_ver_ stty require_controlling_input_terminal_ require_trap_signame_ +require_strace_ ioctl trap '' TTOU # Ignore SIGTTOU @@ -81,4 +82,11 @@ done stty $(cat $saved_state) +# Ensure we validate options before accessing the device +strace -o log1 -e ioctl stty --version || fail=1 +n_ioctl1=$(wc -l < log1) || framework_failure_ +returns_ 1 strace -o log2 -e ioctl stty -blahblah || fail=1 +n_ioctl2=$(wc -l < log2) || framework_failure_ +test "$n_ioctl1" = "$n_ioctl2" || fail=1 + Exit $fail |