nn_mul_kara causes out-of-bounds reads

[percontation]

nn_mul_kara sometimes violates the "bm >= cm" constraint of nn_add_c, potentially leading to invalid memory reads and incorrect results.

The values from this stack trace reveal the issue (i.e. these input parameters to nn_mul_kara seem to meet its input assumptions, but cause a call to nn_add_c that violate nn_add_c's input assumptions):

    frame #4: 0x0000000100034e14 t-nn_subquadratic`nn_add_c(a=0x0000616000007c40, b=0x0000616000007c40, bm=40, c=0x00007fff5fbfe9a0, cm=41, ci=0) + 100 at nn.h:333
    frame #5: 0x0000000100034bb3 t-nn_subquadratic`nn_mul_kara(p=0x0000616000007ba0, a=0x0000613000001020, m=39, b=0x00006110000011e0, n=21) + 4531 at nn_subquadratic.c:63
    frame #6: 0x0000000100002176 t-nn_subquadratic`test_mul_kara + 2710 at t-nn_subquadratic.c:59

Adding ASSERT(bm >= cm) to nn_add_c caused tests failed in multiple places; I'm not sure if nn_mul_kara is the only function violating nn_add_c's input assumptions.

[wbhart]

I'll take a look and see where it's going wrong. Note that I have to work on this in my spare time, of which I have little these days. So unless it's easy to give a correct fix, this might take me a while. I'm of course happy to merge patches though, and see you've submitted a PR.

[percontation]

Yeah, no worries. I might indeed attempt to produce a patch at some point. I just figured I'd try you first in case the fix is easy for someone who already understands the code :)

[wbhart]

I got curious about how such a severe error could have ended up in the code and tracked it down. The nn_mul_kara function was actually doing some work it didn't need to, so I removed that. It's not clear whether I should have left it in, since an extra ASSERT could be added to check the limb that was being computed was in fact zero. But for performance, and to make the code cleaner, I removed it. The extra limb would never be used anywhere, it was just being computed, presumably for the sake of an ASSERT. Let me know what you prefer here. After that, it was strictly using the wrong length for the final addition. I've now fixed this quite severe error in master. I'm actually really surprised this didn't show up in the tests anywhere. It's really rather bad.

[percontation]

Awesome, thanks!

Assumedly the reason it wasn't showing up in the tests is that the high limbs of t and p end up zeroed (I assume?), the former wrong length rarely exceeds the actual allocated lengths, and even when it did run off the end of p it was finding a zero in memory there.