yuripv
mbarden
-
-
usr/src/cmd/grep/grep.c (Diff revision 1) Hmmm, can we move this out of the loop so that we do realloc with needed size just 1 time?
9552 grep segfaults when you ask for context
Review Request #1090 — Created May 24, 2018 and submitted
| Information | |
|---|---|
| andy_js | |
| illumos-gate | |
| 9552 | |
| Reviewers | |
| general | |
You can specify the number of surrounding lines that should also be printed out for each match (with -B NUM, -C NUM or just -NUM) as context, but if you do grep with core dump. This is because the check that tests whether the buffer needs to grow is faulty. Due to missing parenthesis it causes the buffer to balloon prematurely and then later not grow when the buffer is no longer big enough, causing a buffer overflow. It's also possible that after growing it still won't be big enough so we need to do the check in a loop.
The stack trace wasn't much good (it was blowing up in memcpy) so I inserted a bunch of print statements to figure out what was wrong and then fixed the code. No segfaults anymore!
-
Thanks for fixing this.
Rather than do a while() loop allocating BUFSIZE each time, we can just calculate how many multiples of BUFSIZE we need. Something like (((line_len + 1) / BUFSIZE) + 1), rather than just BUFSIZE.
-
-
usr/src/cmd/grep/grep.c (Diff revision 2) We're trying to make (line_len + 1) room in the buffer, in multiples of BUFSIZE. if line_len is exactly BUFSIZE, then we need to allocate at least BUFSIZE+1 bytes. However, this expression will only evaluate to BUFSIZE bytes. While we'll allocate BUFSIZE+1 bytes because we add an extra byte in the realloc expression, conbuflen itself won't represent that final byte (which, unlike the extra byte in realloc, is something actually contained in the 'context buffer').
Should we drop the -1, out of an abundance of caution? At least, a comment here explaining would help.
