8868 /usr/xpg4/bin/grep dumps core in find_nl()
Review Request #785 — Created Nov. 30, 2017 and submitted
Information | |
---|---|
mbarden | |
illumos-gate | |
8868 | |
Reviewers | |
general | |
8868 /usr/xpg4/bin/grep dumps core in find_nl()
New tests were added to the grep_xpg4 test suite for these issues. The rest of the test suite was run for regression testing.
- 3
- 0
- 1
- 1
- 5
Description | From | Last Updated |
---|---|---|
On the first loop through here, data_len will be zero and conptr will be NULL. However, if the context flag ... |
|
|
On our first iteration, before we've assigned conptrend to a non-sentinel value at line 1300, do we really mean for ... |
|
|
This part confused me a little bit. So I tried to put together an example here to try and make ... |
|
-
-
usr/src/cmd/grep_xpg4/grep.c (Diff revision 2) On the first loop through here, data_len will be zero and conptr will be NULL. However, if the context flag isn't set, we won't allocate conbuf at line line 943. This'll mean that conptr will be NULL and conptrend will underflow to ULONG_MAX. Should setting this be conditional on conflag? If not, can we have a comment explaining this case please?
Actually, in general, how is it valid to point to conptr-1? Wouldn't we be storing to illegal memory if we then executed line 1001 on the same iteration?
-
usr/src/cmd/grep_xpg4/grep.c (Diff revision 2) On our first iteration, before we've assigned conptrend to a non-sentinel value at line 1300, do we really mean for this value to result in -1 (if it gets cast to a signed value). As if conbuf was at say 0x1000, won't conptrend at this point be 0xfff? So the subtraction will result in -1? It feels like this won't result in the correct expectations around whether or not we have enough space in the buffer when we're at the initial size and for a very specific value of line_len.
-
usr/src/cmd/grep_xpg4/grep.c (Diff revision 2) This part confused me a little bit. So I tried to put together an
example here to try and make sense of where we are. So let's use this
kind of dummy example.conbuflen: 0x2001 (The default value)
conptrend: 0x0f
conptr: 0x10
line_len: 0x04o This says we memcpy 4 bytes at 0x10.
o We add five to conptrend, so it goes to 0x14
o We clobber the last byte we copied to have a new lineDoes this make sense? I guess the assumption is that we'll always have a
newline character of some kind in the last slot otherwise we wouldn't
have broken out of the growing loop for reading conext, right?Maybe we can write down in a comment somewhere what the invariants or
expectations for conptrend and conptr are and what they are supposed to
point to with respect to being valid or invalid characters? -
usr/src/cmd/grep_xpg4/grep.c (Diff revision 2) I'm a bit confused here on what conptrend is always supposed to point to here. Line 1304
Diff: |
Revision 3 (+107 -39)
|
---|
-
Sorry, guys, I have new core dumps. Run ggrep test suite.
There are more SKIPs and less PASSes. Also I got 2 15-MB core files.
core '/var/cores/grep.26655' of 26655: grep -i ()\1 in
fee8dce2 uselocale (0, 0, 0, 0, 0, 0) + f
fee8e643 mbrtowc (7648064, 7648178, 3, 7648068, fee69b37, 8341c00) + 1e
feea14fd wgetnext (8046000, e, 76480e8, fef52000, 8046000, 7648179) + 56
feea15d6 p_b_symbol (8046000, e, 7648108, fef52000, 8046000, 764817b) + 9e
feea1a95 p_b_term (8046000, 83417e8, 7648160, fef52000, 8046000, fef52000) + 1c5
feea200f p_bracket (8046000, b5, 7648160, fef52000, b5, 764825b) + 15e
feea1dda bothcases (8046000, b5, 76481d8, feea1cb9, 83417a8, ff) + 79
feea1e3f ordinary (8046000, b5, 7648240, fef52000, 8046000, fef52000) + 4d
feea20ba p_bracket (8046000, b5, 7648240, fef52000, b5, 764833b) + 209
feea1dda bothcases (8046000, b5, 76482b8, feea1cb9, 8341768, ff) + 79
feea1e3f ordinary (8046000, b5, 7648320, fef52000, 8046000, fef52000) + 4d
feea20ba p_bracket (8046000, b5, 7648320, fef52000, b5, 764841b) + 209
feea1dda bothcases (8046000, b5, 7648398, feea1cb9, 8341728, ff) + 79
feea1e3f ordinary (8046000, b5, 7648400, fef52000, 8046000, fef52000) + 4d
.....