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;