Unverified Commit 8fb367de authored by Adam Saponara's avatar Adam Saponara
Browse files

prevent use-after-free of `viewport_bline` when reusing a bview.

this bug surfaced as a blank display after `C-q p` or `C-q o`
commands, both of which involve replacing the buffer of an existing
bview. `git bisect` pointed to commit dd283981. stepping through
`buffer_get_bline_w_hint` revealed that we were using a freed bline
as our `opt_hint`, which caused that function to always return
`last_line`, which in turn broke a bunch of stuff. so the bug existed
before this commit, but it was silent. reproducing through valgrind
yielded the following:

```
Invalid read of size 8
   at 0x1375D6: buffer_get_bline_w_hint (buffer.c:715)
   by 0x11B93A: bview_rectify_viewport (bview.c:388)
   by 0x11BE08: bview_resize (bview.c:147)
   by 0x11C41B: bview_open (bview.c:67)
   by 0x1417E5: _cmd_fsearch_inner (cmd.c:1805)
   by 0x13C8A9: cmd_fsearch (cmd.c:605)
   by 0x1238E6: _editor_loop (editor.c:927)
   by 0x11CE51: editor_run (editor.c:153)
   by 0x12EDE0: main (main.c:20)
 Address 0x5257130 is 32 bytes inside a block of size 128 free'd
   at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/...
   by 0x13969F: _buffer_bline_free (buffer.c:1493)
   by 0x135C68: buffer_destroy (buffer.c:205)
   by 0x11B3E4: _bview_deinit (bview.c:599)
   by 0x11C2B8: _bview_init (bview.c:451)
   by 0x11C2B8: bview_open (bview.c:66)
   by 0x1417E5: _cmd_fsearch_inner (cmd.c:1805)
   by 0x13C8A9: cmd_fsearch (cmd.c:605)
   by 0x1238E6: _editor_loop (editor.c:927)
   by 0x11CE51: editor_run (editor.c:153)
   by 0x12EDE0: main (main.c:20)
```

unsetting `viewport_bline` in `_bview_deinit` fixes the bug. other
objects like cursors, marks, srules, etc. which hold bline pointers
are already deinit'd in this function, so we should be covered there.

this is another case where smart pointers might have been useful.

it's hard to write a test for this one as it involves viewport code which
is not executed in headless mode (which tests run in).
parent 38d679db
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment