Check LOI expressions to prevent a read after free.
CS eliminates outer expressions before inner ones, as `x * y * z` before `x * y`. It does this by reversing the order of expressions in the code. This almost always works, but it sometimes doesn't work if a STI changes the value number of a LOI. In code like `expr1 LOI expr2 STI expr2 LOI`, CS might eliminate the inner `expr2` before the outer `expr2 LOI`. This caused a read after free because the occurrence of `expr2 LOI` pointed to the eliminated lines of `expr2`. This bug went unnoticed until my recent changes caused CS to crash with a double free. I did not get the crash in OpenBSD, but I saw the crash in Travis, then David Given reproduced the crash in Linux. See the discussion in https://github.com/davidgiven/ack/pull/73
This commit is contained in:
parent
ebba76e08f
commit
85fcbde22f
|
@ -142,8 +142,9 @@ STATIC void replace(occur_p ocp, offset tmp, avail_p avp)
|
||||||
/* Replace the lines in the occurrence in ocp by a load of the
|
/* Replace the lines in the occurrence in ocp by a load of the
|
||||||
* temporary with offset tmp.
|
* temporary with offset tmp.
|
||||||
*/
|
*/
|
||||||
register line_p lol, first, last;
|
avail_p ravp;
|
||||||
register int instr;
|
line_p lol, first, last;
|
||||||
|
int instr;
|
||||||
|
|
||||||
assert(avp->av_size == ws || avp->av_size == 2*ws);
|
assert(avp->av_size == ws || avp->av_size == 2*ws);
|
||||||
|
|
||||||
|
@ -176,6 +177,33 @@ STATIC void replace(occur_p ocp, offset tmp, avail_p avp)
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Some occurrence rocp of an expression before avp might have
|
||||||
|
* rocp->oc_lfirst == first. If so, then we must set
|
||||||
|
* rocp->oc_lfirst = lol before we throw away first.
|
||||||
|
*
|
||||||
|
* This is almost not possible, but it can happen in code with
|
||||||
|
* expr1 LOI expr2 STI expr2 LOI, where the STI causes both
|
||||||
|
* LOIs to have the same value number. Then the first LOI
|
||||||
|
* might come before the first expr2, so we might replace
|
||||||
|
* expr2 before we replace expr2 LOI. Then the occurrence of
|
||||||
|
* expr2 LOI must not point to the eliminated lines of expr2.
|
||||||
|
*/
|
||||||
|
for (ravp = avp->av_before; ravp != (avail_p) 0;
|
||||||
|
ravp = ravp->av_before) {
|
||||||
|
/* We only check LOI expressions. */
|
||||||
|
if (ravp->av_instr == op_loi) {
|
||||||
|
occur_p rocp;
|
||||||
|
Lindex i;
|
||||||
|
|
||||||
|
for (i = Lfirst(ravp->av_occurs); i != (Lindex) 0;
|
||||||
|
i = Lnext(i, ravp->av_occurs)) {
|
||||||
|
rocp = occ_elem(i);
|
||||||
|
if (rocp->oc_lfirst == first)
|
||||||
|
rocp->oc_lfirst = lol;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/* Throw away the by now useless lines. */
|
/* Throw away the by now useless lines. */
|
||||||
remove_lines(first, last);
|
remove_lines(first, last);
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue