fix variable LLVM shift taint #1346
Conversation
If the item being shifted is a constant but the shift amount is an LLVM variable, taint_mix is called, which calls update_cb, which assumes the constant is always the shift amount. Not only did this produce illogical mask updates, if TAINT2_DEBUG was on and the constant being shifted happened to be -1, PANDA would assert as -1 is the code for not-a-literal.
|
I don't think the failed test has anything to do with the changes I made. I assume it's one of those little hiccups. But I can't find the UI to tell Git to start the checks again. |
|
What's up with the test_installer check? That's twice in a row it has failed. I see on PR 1344 that that test was actually skipped a few days ago. |
|
We've been trying to fix that CI bug over the last few days but it's still sporadically failing - don't worry about that. I was going to hold off on merging for a day or two in case anyone who's more familiar with the taint system wanted to chime in, but this looks good to me. |
|
Also, I just changed some permissions so you might be able to re-run CI yourself from the web UI in the future. |
OK. I was kind of hoping my issue and Mark Mankins' issue would be accepted at the same time anyhow, so I wasn't too disturbed by a little delay. |
That would be great! Is that what the invitation to be a collaborator is about? As long as my employer's security folks won't have a cow, I'll accept. |
|
Yeah, I think the only difference that should matter for you is the ability to click the "re-run failed jobs" button in CI - technically you can also make your own branches on the main repo too, but maybe it's best to stick with your current workflow. |
|
Good - that was my intention - pretend my only new power is to restart CI checks. |
|
Feel free to restart CI anytime! It often needs a good kick, though we are trying to make it a bit more stable. |
The taint instrumentation code inserts a taint_mix operation whenever it encounters an LLVM shift instruction with one constant operand and one variable operand. The taint_mix function used to call the update_cb function to update the masks. The update_cb function would assume that the constant operand is the shift amount. But, sometimes it is not, which would result in non-sense updates to the masks. Even worse, if PANDA was built with TAINT2_DEBUG and that was enabled, if the constant being shifted was -1, then PANDA would assert (because -1 is the code for not-a-literal).
I wasn't sure what a sensible way would be to update the masks in such a case (unless all the bits of the shift amount were tainted, which rarely happens), so instead I took the "bail out" approach used for other LLVM instructions when it's too difficult to figure out how to update the masks. This means the labels are propagated to the destination, but all 3 masks (cb, zero, ones) are cleared.
If the item being shifted is the variable and the shift amount is the constant, then the processing is unchanged - the masks get updated same as before.