OK martijn@ for the pattern.c part.
The search.c part seems unneeded.
- If we don't have a match we don't enter hilite_line at all.
- If we have a match and don't negate get sp/ep for every match.
- If we have a match and negate, with your process.c patch we get a
sp/ep with value NULL and return before entering the do/while.
So with the above there's no situation where we can get in the
do/while loop and still need to check for NULL on every pass, and the
move is thus unnecessary churn.
On 07/28/17 13:32, Anton Lindqvist wrote:
> On Fri, Jul 28, 2017 at 01:19:39PM +0200, Theo Buehler wrote:
>> On Thu, Jul 27, 2017 at 10:31:51AM +0100, Larry Hynes wrote:
>>> Hi
>>>
>>> $ env | grep LESS
>>> LESSHISTFILE=-
>>> LESS="-i -M -R -g -c"
>>> LESSCHARSET=utf-8
>>>
>>> $ unset LESS
>>> $ unset LESSCHARSET
>>> $ unset LESSHISTFILE
>>>
>>> $ LESS="-g"
>>> $ echo 'foo\nbar\nfoo\nbar\nfoo\nbar' | less
>>>
>>> While in less, '/' to search, then '^N', or '!', to search for lines
>>> which do NOT match the pattern, entering foo<CR> results in a seg
>>> fault.
>>>
>>> This is on amd64.
>>
>> Thanks for the report. Of course you want 'export LESS="-g"' here,
>> otherwise less(1) won't use the -g flag which is crucial for this crash.
>>
>> I don't have a fix, but maybe this helps a bit:
>>
>> From the command line, without environment variable magic and no
>> interactive component you can reproduce as follows:
>>
>> $ echo 'foo\nbar\nfoo\n' > crash
>> $ less -g -p '!foo' crash
>>
>> It's apparently a use-after-free that happens in search_range():
>>
>> Program terminated with signal SIGSEGV, Segmentation fault.
>> #0 0x000009d426e1b35b in create_hilites (linepos=4, start_index=-370944992,
>> end_index=-370944989, chpos=0x9d6c5994c80)
>> at /usr/src/usr.bin/less/less/../search.c:445
>> 445 hl->hl_startpos = linepos + chpos[start_index];
>> (gdb) bt
>> #0 0x000009d426e1b35b in create_hilites (linepos=4, start_index=-370944992,
>> end_index=-370944989, chpos=0x9d6c5994c80)
>> at /usr/src/usr.bin/less/less/../search.c:445
>> #1 0x000009d426e1b24c in hilite_line (linepos=4, line=0x9d65872bf70 "bar",
>> line_len=3, chpos=0x9d6c5994c80,
>> sp=0x9d642569390 '\337' <repeats 15 times>, <incomplete sequence \337>,
>> ep=0x9d642569393 '\337' <repeats 12 times>, <incomplete sequence \337>)
>> at /usr/src/usr.bin/less/less/../search.c:494
>> #2 0x000009d426e1aa99 in search_range (pos=8, endpos=-1, search_type=257,
>> matches=0, maxlines=-1, plinepos=0x7f7ffffe9ff0,
>> pendpos=0x0) at /usr/src/usr.bin/less/less/../search.c:804
>> #3 0x000009d426e1a258 in search (search_type=257, pattern=0x9d42702ca40
>> <cmdbuf> "foo", n=1)
>> at /usr/src/usr.bin/less/less/../search.c:925
>> #4 0x000009d426e0a182 in multi_search (pattern=0x9d42702ca40 <cmdbuf>
>> "foo", n=1) at /usr/src/usr.bin/less/less/../command.c:815
>> #5 0x000009d426e0a779 in exec_mca () at
>> /usr/src/usr.bin/less/less/../command.c:189
>> #6 0x000009d426e09d03 in mca_char (c=10) at
>> /usr/src/usr.bin/less/less/../command.c:532
>> #7 0x000009d426e08865 in commands () at
>> /usr/src/usr.bin/less/less/../command.c:971
>> #8 0x000009d426e010f2 in main (argc=-1, argv=0x7f7ffffea200) at
>> /usr/src/usr.bin/less/less/../main.c:274
>>
>
> I took a closer look earlier today. As I see it, {start,end}_index does
> not get updated inside match_pattern() since we're looking for inverted
> matches, i.e. when regexec() indicates no match we actually got a match.
> Instead, the indices could be set to NULL up front and the corresponding
> NULL-check moved into the loop since match_pattern() is called
> repeatedly. Tested with and without `-g` for both normal and inverted
> search.
>
> Index: pattern.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/pattern.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 pattern.c
> --- pattern.c 21 Nov 2015 13:29:12 -0000 1.9
> +++ pattern.c 28 Jul 2017 11:31:34 -0000
> @@ -122,6 +122,8 @@ match_pattern(void *pattern, char *tpatt
> rm.rm_so = 0;
> rm.rm_eo = line_len;
> #endif
> + *sp = NULL;
> + *ep = NULL;
> matched = !regexec(spattern, line, 1, &rm, flags);
> if (matched) {
> *sp = line + rm.rm_so;
> Index: search.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/search.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 search.c
> --- search.c 17 Sep 2016 15:06:41 -0000 1.18
> +++ search.c 28 Jul 2017 11:31:34 -0000
> @@ -477,8 +477,6 @@ hilite_line(off_t linepos, char *line, i
> char *searchp;
> char *line_end = line + line_len;
>
> - if (sp == NULL || ep == NULL)
> - return;
> /*
> * sp and ep delimit the first match in the line.
> * Mark the corresponding file positions, then
> @@ -491,6 +489,9 @@ hilite_line(off_t linepos, char *line, i
> */
> searchp = line;
> do {
> + if (sp == NULL || ep == NULL)
> + return;
> +
> create_hilites(linepos, (intptr_t)sp - (intptr_t)line,
> (intptr_t)ep - (intptr_t)line, chpos);
> /*
>
