x/crypto/bn256: TestG1Marshal fails with Go tip

[mundaym]

This package calls math/big.ModInverse and does not check that the result is non-nil, which appears to result in a segmentation fault when it is tested against Go tip.

See https://storage.googleapis.com/go-build-log/9ecf899b/freebsd-amd64-11_1_a2ef6f73.log for an example.

[mundaym]

Presumably a result of CL 108996.

cc @bmkessler @griesemer @FiloSottile

[bradfitz]

Presumably a result of CL 108996.

Yes, it was CL 108996.

[griesemer]

diff --git a/bn256/curve.go b/bn256/curve.go
index 55b7063..9b8ff23 100644
--- a/bn256/curve.go
+++ b/bn256/curve.go
@@ -251,6 +251,9 @@ func (c *curvePoint) MakeAffine(pool *bnPool) *curvePoint {
        }
 
        zInv := pool.Get().ModInverse(c.z, p)
+       if zInv == nil {
+               zInv = pool.Get()
+       }
        t := pool.Get().Mul(c.y, zInv)
        t.Mod(t, p)
        zInv2 := pool.Get().Mul(zInv, zInv)

essentially restores the old behavior and makes the test pass. But I don't know if this is the correct fix. (If the ModInverse does not exist, in the past the returned value was undefined/the behavior of ModInverse was not specified.)

@agl ?

[gopherbot]

Change https://golang.org/cl/110695 mentions this issue: bn256: skip converting to affine coordinates for ∞

[FiloSottile]

I honestly have no idea how it used to work. I think it relied on the fact that pool.Get() was returning 0, and that ModInverse was returning 0 as well.