On Tue, Oct 17, 2017 at 12:37 AM, Pádraig Brady <p...@draigbrady.com> wrote:
> On 16/10/17 10:49, Jim Meyering wrote:
>> On Mon, Oct 16, 2017 at 2:30 AM, Pádraig Brady <p...@draigbrady.com> wrote:
>>> On 15/10/17 18:07, Jaeseung Choi wrote:
>>>> Dear GNU team,
>>>>
>>>> While testing coreutils for a research purpose, we found the following
>>>> crash in 'stty'. Running stty with the command-line "stty eol -F AA"
>>>> raises a crash as below. We did not change any terminal setting, and
>>>> believe the bug is irrelevant from any specific terminal
>>>> configuration.
>>>>
>>>> jason@ubuntu:~$ tar -xf coreutils-8.28.tar.xz
>>>> jason@ubuntu:~$ cd coreutils-8.28/
>>>> jason@ubuntu:~/coreutils-8.28$ mkdir obj
>>>> jason@ubuntu:~/coreutils-8.28$ cd obj
>>>> jason@ubuntu:~/coreutils-8.28/obj$ ../configure --disable-nls && make
>>>> ...
>>>> jason@ubuntu:~/coreutils-8.28/obj$ gdb ./src/stty -q
>>>> Reading symbols from ./src/stty...done.
>>>> (gdb) run eol -F AA
>>>> Starting program: /home/jason/coreutils-8.28/obj/src/stty eol -F AA
>>>>
>>>> Program received signal SIGSEGV, Segmentation fault.
>>>> set_control_char (info=0x40a6f8 <control_info+120>, info=0x40a6f8
>>>> <control_info+120>, mode=0x6103c0 <check_mode>, arg=0x0) at
>>>> ../src/stty.c:1695
>>>> 1695 else if (arg[0] == '\0' || arg[1] == '\0')
>>>> (gdb) x/i $rip
>>>> => 0x40387a <apply_settings+746>: movzbl (%rbx),%r14d
>>>> (gdb) info reg rbx
>>>> rbx 0x0 0
>>>> (gdb)
>>>>
>>>> We could reproduce the bug in coreutils from version 8.27 to 8.28.
>>>> Also, the bug was reproducible in both Ubuntu 16.04 and Debian 9.1.
>>>> But the stty program pre-built in Debian 9.1 did not crash because
>>>> currently 8.26 version is installed in Debian.
>>>
>>> This is actually an old bug which you can reproduce with -F /dev/tty.
>>> The attached should fix it up.
>>
>> Thank you!
>> If it's not too hard to determine, would you please mention in the log
>> the commit that introduced the bug?
>
> Updated patch attached. I mistakenly thought getopt would
> permute the argv so NULLs were at the end. The attached
> caters for NULLs interspersed in the argv[].
Good catch!
One suggestion: indent the backslashes to column 72, e.g., with this patch:
diff --git a/src/stty.c b/src/stty.c
index 1a5c1e962..29f78375a 100644
--- a/src/stty.c
+++ b/src/stty.c
@@ -1089,11 +1089,11 @@ apply_settings (bool checking, const char *device_name,
struct termios *mode, bool *speed_was_set,
bool *require_set_attr)
{
-#define check_argument(arg) \
- if (k == n_settings - 1 || ! settings[k+1]) \
- { \
- error (0, 0, _("missing argument to %s"), quote (arg)); \
- usage (EXIT_FAILURE); \
+#define check_argument(arg) \
+ if (k == n_settings - 1 || ! settings[k+1]) \
+ { \
+ error (0, 0, _("missing argument to %s"), quote (arg)); \
+ usage (EXIT_FAILURE); \
}
for (int k = 1; k < n_settings; k++)
