Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIGSEGV when adding a % to user-commands with preview and compete = "file" options. #28851

Open
ayosec opened this issue May 19, 2024 · 1 comment
Assignees
Labels
bug-crash issue reporting a crash or segfault cmdline-mode command line, also cmdwin has:backtrace issue contains a stacktrace/ASAN log
Milestone

Comments

@ayosec
Copy link

ayosec commented May 19, 2024

Problem

When the character % is added to the Ex command-line, and the command in the prompt is defined with the options preview and complete = "file" (via nvim_create_user_command), Neovim crashes with a memory error.

The crash happens only if the current buffer has a file. If expand("%") is empty it seems to work.

Sometimes it crashes with a SIGSEGV (with no error messages), and some other times it prints E41: Out of memory! and dies.

I found the issue with complete = "file", but it also fails with "dir". Other completions (like, "help", "syntax", or "file_in_path") have no problems.

I tested multiple Neovim versions (Linux x86_64, from Nix and from the prebuilt packages in https://github.com/neovim/neovim/releases), and it seems that it is broken since 0.8 (when preview was added).

Steps to reproduce

  1. Create a user-command with complete = "file" and preview. The function for preview can be empty.

    local function callback()
    end
    
    local function preview()
        return 0
    end
    
    vim.api.nvim_create_user_command("TestCommand", callback, {
        nargs = "?",        -- or "*"
        complete = "file",  -- or "dir"
        preview = preview,
    })
  2. Ensure that the current buffer has a file (so expand("%") is not empty).

    :e /tmp/testfile
  3. Open the command-line and type the command with any argument without a %.

    The completion will work as expected.

    :TestCommand /tmp/<Tab>
  4. Now, write a % in the command-line. Neovim crashes immediately.

    :TestCommand /tmp/%

The steps can be automated:

  1. A Lua script to create the user-command, open a file, and simulate the :TestCommand % input:

    local function callback()
    end
    
    local function preview()
        return 0
    end
    
    vim.api.nvim_create_user_command("TestCommand", callback, {
        nargs = "?",        -- or "*"
        complete = "file",  -- or "dir"
        preview = preview,
    })
    
    
    
    vim.defer_fn(
        function()
            -- Open any file.
            vim.cmd.edit("/tmp/test" .. os.time())
    
            -- Type the command with a `%`.
            vim.api.nvim_feedkeys(":TestCommand %", "n", false)
        end,
        200
    )
  2. Then, open Neovim with it:

    $ nvim --clean -c 'so test-sigsegv.lua'

Expected behavior

No crashes.

Neovim version (nvim -v)

0.10.0

Vim (not Nvim) behaves the same?

This is exclusive to Neovim

Operating system/version

Debian 12

Terminal name/version

Alacritty

$TERM environment variable

alacritty

Installation

Prebuilt packages and nixpkgs

@ayosec ayosec added the bug issues reporting wrong behavior label May 19, 2024
@ayosec
Copy link
Author

ayosec commented May 19, 2024

I did some debugging against the commit 0f4f7d3.

When a % is pressed the SIGSEGV is caught in GDB.

Full Backtrace
Thread 1 "nvim" received signal SIGSEGV, Segmentation fault.
0x00007f1c60a7fc01 in __memmove_avx_unaligned ()
   from /nix/store/apab5i73dqa09wx0q27b6fbhd1r18ihl-glibc-2.39-31/lib/libc.so.6
(gdb) bt
#0  0x00007f1c60a7fc01 in __memmove_avx_unaligned ()
   from /nix/store/apab5i73dqa09wx0q27b6fbhd1r18ihl-glibc-2.39-31/lib/libc.so.6
#1  0x000000000052de2c in memmove (__len=<optimized out>, __src=<optimized out>, __dest=<optimized out>,
    __dest=<optimized out>, __src=<optimized out>, __len=<optimized out>)
    at /nix/store/s3pvsv4as7mc8i2nwnk2hnsyi2qdj4bq-glibc-2.39-31-dev/include/bits/string_fortified.h:36
#2  repl_cmdline (eap=eap@entry=0x7ffea1c01120, src=<optimized out>, srclen=1,
    repl=repl@entry=0x9f2a20 "/tmp/test1716143731", cmdlinep=cmdlinep@entry=0x7ffea1c01098)
    at /home/nix/binaries/neovim/src/nvim/ex_docmd.c:4025
#3  0x0000000000536009 in expand_filename (eap=0x7ffea1c01120, cmdlinep=0x7ffea1c01098, errormsgp=0x7ffea1c00ab8)
    at /home/nix/binaries/neovim/src/nvim/ex_docmd.c:3943
#4  0x00000000005289d3 in execute_cmd0 (retv=retv@entry=0x7ffea1c00ab4, eap=eap@entry=0x7ffea1c01120,
    errormsg=errormsg@entry=0x7ffea1c00ab8, preview=preview@entry=true)
    at /home/nix/binaries/neovim/src/nvim/ex_docmd.c:1643
#5  0x0000000000528ffe in execute_cmd (eap=0x7ffea1c01120, cmdinfo=<optimized out>, preview=<optimized out>)
    at /home/nix/binaries/neovim/src/nvim/ex_docmd.c:1791
#6  0x000000000053da9e in cmdpreview_may_show (s=s@entry=0x7ffea1c01730)
    at /home/nix/binaries/neovim/src/nvim/ex_getln.c:2573
#7  0x000000000053dd46 in command_line_changed (s=s@entry=0x7ffea1c01730)
    at /home/nix/binaries/neovim/src/nvim/ex_getln.c:2657
#8  0x0000000000540671 in command_line_handle_key (s=s@entry=0x7ffea1c01730)
    at /home/nix/binaries/neovim/src/nvim/ex_getln.c:2171
#9  0x0000000000540ddf in command_line_execute (state=0x7ffea1c01730, key=<optimized out>)
    at /home/nix/binaries/neovim/src/nvim/ex_getln.c:1390
#10 0x0000000000683ebd in state_enter (s=0x7ffea1c01730) at /home/nix/binaries/neovim/src/nvim/state.c:101
#11 0x000000000053e3dc in command_line_enter (firstc=firstc@entry=58, count=count@entry=1, indent=indent@entry=0,
    clear_ccline=clear_ccline@entry=true) at /home/nix/binaries/neovim/src/nvim/ex_getln.c:842
#12 0x000000000053e813 in getcmdline (firstc=firstc@entry=58, count=count@entry=1, indent=indent@entry=0,
    do_concat=do_concat@entry=true) at /home/nix/binaries/neovim/src/nvim/ex_getln.c:2719
#13 0x000000000053ec0f in getexline (c=58, cookie=<optimized out>, indent=0, do_concat=true)
    at /home/nix/binaries/neovim/src/nvim/ex_getln.c:2962
#14 0x000000000052a79c in do_cmdline (cmdline=<optimized out>, fgetline=0x53ebe6 <getexline>, cookie=0x0, flags=0)
    at /home/nix/binaries/neovim/src/nvim/ex_docmd.c:578
#15 0x00000000005e6cb5 in nv_colon (cap=0x7ffea1c020f0) at /home/nix/binaries/neovim/src/nvim/normal.c:3179
#16 0x00000000005df741 in normal_execute (state=0x7ffea1c02080, key=<optimized out>)
    at /home/nix/binaries/neovim/src/nvim/normal.c:1229
#17 0x0000000000683ebd in state_enter (s=0x7ffea1c02080) at /home/nix/binaries/neovim/src/nvim/state.c:101
#18 0x00000000005de16f in normal_enter (cmdwin=false, noexmode=false)
    at /home/nix/binaries/neovim/src/nvim/normal.c:518
#19 0x00000000005a0001 in main (argc=7, argv=<optimized out>) at /home/nix/binaries/neovim/src/nvim/main.c:664

The SIGSEGV is triggered in the ex_docmd.c:4025:

memmove(new_cmdline, *cmdlinep, i);

It seems that *cmdlinep is NULL:

(gdb) frame 2
#2  repl_cmdline (eap=eap@entry=0x7ffea1c01120, src=<optimized out>, srclen=1,
    repl=repl@entry=0x9f2a20 "/tmp/test1716143731", cmdlinep=cmdlinep@entry=0x7ffea1c01098)
    at /home/nix/binaries/neovim/src/nvim/ex_docmd.c:4025
4025	  memmove(new_cmdline, *cmdlinep, i);

(gdb) print new_cmdline
$1 = 0x7f8979b64010 ""

(gdb) print cmdlinep
$2 = (char **) 0x7ffd18f88318

(gdb) print *cmdlinep
$3 = 0x0

(gdb) print i
$4 = 28152748

In the function cmdpreview_may_show(), the expression ea.cmdlinep has the same value:

(gdb) frame 6
#6  0x000000000053da9e in cmdpreview_may_show (s=s@entry=0x7ffd18f889b0)
    at /home/nix/binaries/neovim/src/nvim/ex_getln.c:2573
2573	  cmdpreview_type = execute_cmd(&ea, &cmdinfo, true);

(gdb) print ea.cmdlinep
$5 = (char **) 0x7ffd18f88318

(gdb) print *ea.cmdlinep
$6 = 0x0

The field cmdlinep is initialized in parse_cmdline() (which is called by cmdpreview_may_show() before the crash), but it takes the address of a local variable:

neovim/src/nvim/ex_docmd.c

Lines 1471 to 1490 in 0f4f7d3

bool parse_cmdline(char *cmdline, exarg_T *eap, CmdParseInfo *cmdinfo, const char **errormsg)
{
char *after_modifier = NULL;
bool retval = false;
// parsing the command modifiers may set ex_pressedreturn
const bool save_ex_pressedreturn = ex_pressedreturn;
// parsing the command range may require moving the cursor
const pos_T save_cursor = curwin->w_cursor;
// parsing the command range may set the last search pattern
save_last_search_pattern();
// Initialize cmdinfo
CLEAR_POINTER(cmdinfo);
// Initialize eap
*eap = (exarg_T){
.line1 = 1,
.line2 = 1,
.cmd = cmdline,
.cmdlinep = &cmdline,

Maybe there is something I'm missing, but the address stored in cmdlinep should not be used after parse_cmdline() returns.

I put a breakpoint in parse_cmdline() right after initialing the *eap variable.

(gdb) break ex_docmd.c:1495
Breakpoint 1 at 0x5285af: file /home/nix/binaries/neovim/src/nvim/ex_docmd.c, line 1496.

(gdb) c
Continuing.

Thread 1 "nvim" hit Breakpoint 1, parse_cmdline (cmdline=<optimized out>, eap=0x7ffef953f680, cmdinfo=0x7ffef953f740,
    errormsg=0x7ffef953f658) at /home/nix/binaries/neovim/src/nvim/ex_docmd.c:1496
1496	  if (parse_command_modifiers(eap, errormsg, &cmdinfo->cmdmod, false) == FAIL) {

(gdb) print eap.cmdlinep
$1 = (char **) 0x7ffef953f5f8

(gdb) print *eap.cmdlinep
$2 = 0x1e7b740 "TestCommand %"

The value in cmdlinep is an address in the stack, and the memory pointed by *cmdlinep is on the heap:

(gdb) info proc mappings

          Start Addr           End Addr       Size     Offset  Perms  objfile
[...]
           0x1e6b000          0x1f6f000   0x104000        0x0  rw-p   [heap]
[...]
      0x7ffef9521000     0x7ffef9543000    0x22000        0x0  rw-p   [stack]

After returning from parse_cmdline the value in ea.cmdlinep is still valid until it calls to cmdpreview_prepare:

(gdb) finish
Run till exit from #0  parse_cmdline (cmdline=<optimized out>, eap=0x7ffef953f680, cmdinfo=0x7ffef953f740,
    errormsg=0x7ffef953f658) at /home/nix/binaries/neovim/src/nvim/ex_docmd.c:1496
0x000000000053d9be in cmdpreview_may_show (s=s@entry=0x7ffef953fc90)
    at /home/nix/binaries/neovim/src/nvim/ex_getln.c:2522
2522	  if (!parse_cmdline(cmdline, &ea, &cmdinfo, &errormsg)) {
Value returned is $5 = true

(gdb) display *ea.cmdlinep
1: *ea.cmdlinep = 0x1e7b740 "TestCommand %"

(gdb) next
2526	  emsg_off--;
1: *ea.cmdlinep = 0x1e7b740 "TestCommand %"

[...]

(gdb)
2549	  block_autocmds();              // Block events
1: *ea.cmdlinep = 0x1e7b740 "TestCommand %"

(gdb)
2552	  cmdpreview_prepare(&cpinfo);
1: *ea.cmdlinep = 0x1e7b740 "TestCommand %"

(gdb)
2555	  if (icm_split && (cmdpreview_buf = cmdpreview_open_buf()) == NULL) {
1: *ea.cmdlinep = 0x0

block_autocmds() has no local variables, so maybe it is just a coincidence that the value is not overwritten until the call to cmdpreview_prepare().

Possible Fix

If I reinitialize the cmdlinep field after the call to parse_cmdline(), with the address of cmdline in cmdpreview_may_show() (the same action done by parse_cmdline(), but with a variable valid during the execution of cmdpreview_may_show()), then I can't reproduce the crash anymore.

diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c
index 8c9e6e45..a88e9651 100644
--- a/src/nvim/ex_getln.c
+++ b/src/nvim/ex_getln.c
@@ -2525,6 +2525,8 @@ static bool cmdpreview_may_show(CommandLineState *s)
   }
   emsg_off--;

+  ea.cmdlinep = &cmdline;
+
   // Check if command is previewable, if not, don't attempt to show preview
   if (!(ea.argt & EX_PREVIEW)) {
     undo_cmdmod(&cmdinfo.cmdmod);

I didn't sent a pull-request because I'm not sure if this is a valid solution.

Address Sanitizer

After finding the issue, I realized that this problem should be visible by ASAN. I recompiled Neovim with -DENABLE_ASAN_UBSAN=1 (without the possible fix), and it reports a stack-use-after-return.

ASAN Report
=================================================================
==68834==ERROR: AddressSanitizer: stack-use-after-return on address 0x7f9eb095ada0 at pc 0x000000814d03 bp 0x7fffeaf113f0 sp 0x7fffeaf113e8
READ of size 8 at 0x7f9eb095ada0 thread T0
    #0 0x814d02 in repl_cmdline /home/nix/binaries/neovim/src/nvim/ex_docmd.c:4013
    #1 0x82305a in expand_filename /home/nix/binaries/neovim/src/nvim/ex_docmd.c:3943
    #2 0x80635c in execute_cmd0 /home/nix/binaries/neovim/src/nvim/ex_docmd.c:1643
    #3 0x8080a2 in execute_cmd /home/nix/binaries/neovim/src/nvim/ex_docmd.c:1791
    #4 0x871216 in cmdpreview_may_show /home/nix/binaries/neovim/src/nvim/ex_getln.c:2575
    #5 0x8776d4 in command_line_changed /home/nix/binaries/neovim/src/nvim/ex_getln.c:2659
    #6 0x861ad6 in command_line_handle_key /home/nix/binaries/neovim/src/nvim/ex_getln.c:2171
    #7 0x863dff in command_line_execute /home/nix/binaries/neovim/src/nvim/ex_getln.c:1390
    #8 0xde824e in state_enter /home/nix/binaries/neovim/src/nvim/state.c:101
    #9 0x8593c5 in command_line_enter /home/nix/binaries/neovim/src/nvim/ex_getln.c:842
    #10 0x86723a in getcmdline /home/nix/binaries/neovim/src/nvim/ex_getln.c:2721
    #11 0x86cafa in getexline /home/nix/binaries/neovim/src/nvim/ex_getln.c:2964
    #12 0x80de96 in do_cmdline /home/nix/binaries/neovim/src/nvim/ex_docmd.c:578
    #13 0xb2c5ee in nv_colon /home/nix/binaries/neovim/src/nvim/normal.c:3179
    #14 0xb22a05 in normal_execute /home/nix/binaries/neovim/src/nvim/normal.c:1229
    #15 0xde824e in state_enter /home/nix/binaries/neovim/src/nvim/state.c:101
    #16 0xb12ac2 in normal_enter /home/nix/binaries/neovim/src/nvim/normal.c:518
    #17 0x9e57df in main /home/nix/binaries/neovim/src/nvim/main.c:664
    #18 0x7f9eb287410d in __libc_start_call_main (/nix/store/apab5i73dqa09wx0q27b6fbhd1r18ihl-glibc-2.39-31/lib/libc.so.6+0x2a10d) (BuildId: cfd0adeaaed7d46a379028cc9694fc95dd125976)
    #19 0x7f9eb28741c8 in __libc_start_main_alias_1 (/nix/store/apab5i73dqa09wx0q27b6fbhd1r18ihl-glibc-2.39-31/lib/libc.so.6+0x2a1c8) (BuildId: cfd0adeaaed7d46a379028cc9694fc95dd125976)
    #20 0x46dba4 in _start (/home/nix/binaries/install/bin/nvim+0x46dba4)

Address 0x7f9eb095ada0 is located in stack of thread T0 at offset 32 in frame
    #0 0x804a42 in parse_cmdline /home/nix/binaries/neovim/src/nvim/ex_docmd.c:1472

  This frame has 2 object(s):
    [32, 40) 'cmdline' (line 1471) <== Memory access at offset 32 is inside this variable
    [64, 76) 'save_cursor' (line 1478)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-use-after-return /home/nix/binaries/neovim/src/nvim/ex_docmd.c:4013 in repl_cmdline
Shadow bytes around the buggy address:
  0x7f9eb095ab00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 00 00 00 00
  0x7f9eb095ab80: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 00 00 00 00
  0x7f9eb095ac00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 00 00 00 00
  0x7f9eb095ac80: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 00 00 00 00
  0x7f9eb095ad00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 00 00 00 00
=>0x7f9eb095ad80: f5 f5 f5 f5[f5]f5 f5 f5 f5 f5 f5 f5 00 00 00 00
  0x7f9eb095ae00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 00 00 00 00
  0x7f9eb095ae80: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 00 00 00 00
  0x7f9eb095af00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 00 00 00 00
  0x7f9eb095af80: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x7f9eb095b000: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==68834==ABORTING

With the possible fix, ASAN does not report any problem.

@zeertzjq zeertzjq added has:backtrace issue contains a stacktrace/ASAN log bug-crash issue reporting a crash or segfault cmdline-mode command line, also cmdwin and removed bug issues reporting wrong behavior labels May 19, 2024
@justinmk justinmk added this to the backlog milestone May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-crash issue reporting a crash or segfault cmdline-mode command line, also cmdwin has:backtrace issue contains a stacktrace/ASAN log
Projects
None yet
Development

No branches or pull requests

4 participants