Klemens Nanni <k...@openbsd.org> writes:
> I fat fingered commands and it crashed. Here is a reproducer
> (files do not have to exist):
>
> $ vi foo
> :e
> :e bar
> :q!
> vi(12918) in free(): write after free 0xea559a2d980
> Abort trap (core
> dumped)
>
> In words: open a file, open an empty file, open another file, exit
> forcefully.
>
> Here's a backtrace produced with a DEBUG='-g3 -O0' exectuable:
>
> #0 thrkill () at /tmp/-:3
> 3 /tmp/-: No such file or directory.
> #0 thrkill () at /tmp/-:3
> #1 0x00000f8c41ddb78e in _libc_abort () at
> /usr/src/lib/libc/stdlib/abort.c:51
> #2 0x00000f8c41d8e096 in wrterror (d=0xf8c0ff999e0, msg=0xf8c41d6c911 "write
> after free %p") at /usr/src/lib/libc/stdlib/malloc.c:307
> #3 0x00000f8c41d8ee1a in ofree (argpool=0x7f7fffff3dc0, p=<optimized out>,
> clear=<optimized out>, check=<optimized out>, argsz=<optimized out>) at
> /usr/src/lib/libc/stdlib/malloc.c:1439
> #4 0x00000f8c41d8e2db in free (ptr=0xf8bcf80a600) at
> /usr/src/lib/libc/stdlib/malloc.c:1470
> #5 0x00000f89c487c803 in opts_free (sp=0xf8c03c1e7a0) at
> /usr/src/usr.bin/vi/build/../common/options.c:1096
> #6 0x00000f89c4880936 in screen_end (sp=0xf8c03c1e7a0) at
> /usr/src/usr.bin/vi/build/../common/screen.c:192
> #7 0x00000f89c489a013 in vi (spp=0x7f7fffff41d8) at
> /usr/src/usr.bin/vi/build/../vi/vi.c:257
> #8 0x00000f89c4875a4b in editor (gp=0xf8c5dfc85f0, argc=1,
> argv=0x7f7fffff4320) at /usr/src/usr.bin/vi/build/../common/main.c:429
> #9 0x00000f89c484566b in main (argc=2, argv=0x7f7fffff4318) at
> /usr/src/usr.bin/vi/build/../cl/cl_main.c:97
>
>
> I have no time to look at this myself, feel free to take over.
Did a little digging...this diff (with extra context to help explain)
fixes it for me, but I haven't tested much of a workflow other than what
was breaking.
What I'm seeing is if a user is editing a named file that's backed only
by a temp file and not yet persisted, when executing the ex_edit command
(:e) with no arg it ends up an err path in exf.c:file_init() shown here:
381 err:
382 free(frp->name);
383 frp->name = NULL;
384 if (frp->tname != NULL) {
385 (void)unlink(frp->tname);
386 free(frp->tname);
387 frp->tname = NULL;
388 }
We end up freeing some strings and unlinking the temp file. You can
easily see this without a debugger by checking /tmp before and after the
reproduction step of an arg-less ':e'.
-dv
diff 8095b13035d3c80c255344b9166e7f4ff88e61e3 /usr/src
blob - 0b6ae026533e5696a31f4bd87291ccd1d7d5e58f
file + usr.bin/vi/common/exf.c
--- usr.bin/vi/common/exf.c
+++ usr.bin/vi/common/exf.c
@@ -170,12 +170,20 @@ file_init(SCR *sp, FREF *frp, char *rcv_name, int flag
* If no name or backing file, for whatever reason, create a backing
* temporary file, saving the temp file name so we can later unlink
* it. If the user never named this file, copy the temporary file name
* to the real name (we display that until the user renames it).
*/
oname = frp->name;
+
+ /*
+ * User is editing a name file that doesn't exist yet other than as a
+ * temporary file.
+ */
+ if (!exists && oname != NULL && frp->tname != NULL)
+ return (1);
+
if (LF_ISSET(FS_OPENERR) || oname == NULL || !exists) {
/*
* Don't try to create a temporary support file twice.
*/
if (frp->tname != NULL)
goto err;
