Merge pull request #97 from exarkun/96.decoder-crash

Fix decoding with k == m == 256

Author: exarkun

changelog

@@ -1,3 +1,6 @@
+Wed Sep 20 10:35:00 EST 2023 exarkun@twistedmatrix.com
+  * zfec: fix incorrect results, memory corruption, and a sometimes-crash when decoding with k = n = 256.
+
 Tue Jan 25 13:45:00 EST 2022 exarkun@twistedmatrix.com
   * zfec: setup: remove support for python < 3.7
 

setup.py

@@ -58,7 +58,7 @@
     url="https://github.com/tahoe-lafs/zfec",
     extras_require={
         "bench": ["pyutil >= 3.0.0"],
-        "test": ["twisted", "pyutil >= 3.0.0"],
+        "test": ["twisted", "pyutil >= 3.0.0", "hypothesis"],
     },
     ext_modules=extensions,
     cmdclass=versioneer.get_cmdclass(),

zfec/fec.c

@@ -511,7 +511,7 @@ fec_encode(const fec_t* code, const gf*restrict const*restrict const src, gf*res
  */
 void
 build_decode_matrix_into_space(const fec_t*restrict const code, const unsigned*const restrict index, const unsigned k, gf*restrict const matrix) {
-    unsigned char i;
+    unsigned short i;
     gf* p;
     for (i=0, p=matrix; i < k; i++, p += k) {
         if (index[i] < k) {
@@ -527,9 +527,22 @@ build_decode_matrix_into_space(const fec_t*restrict const code, const unsigned*c
 void
 fec_decode(const fec_t* code, const gf*restrict const*restrict const inpkts, gf*restrict const*restrict const outpkts, const unsigned*restrict const index, size_t sz) {
     gf* m_dec = (gf*)alloca(code->k * code->k);
+
+    /* char is large enough for outix - it counts the number of primary blocks
+       we are decoding for return.  the most primary blocks we might have to
+       decode is for k == 128, m == 256.  in this case we might be given 128
+       secondary blocks and have to decode 128 primary blocks.  if k decreases
+       then the number of total blocks we might have to return decreases.  if
+       k increases then the number of secondary blocks that exist decreases so
+       we will be passed some primary blocks and the number of primary blocks
+       we have to decode decreases. */
     unsigned char outix=0;
-    unsigned char row=0;
-    unsigned char col=0;
+
+    /* row and col are compared directly to k, which could be 256, so make
+       them large enough to represent 256.
+     */
+    unsigned short row=0;
+    unsigned short col=0;
     build_decode_matrix_into_space(code, index, code->k, m_dec);
 
     for (row=0; row<code->k; row++) {

zfec/test/test_zfec.py

@@ -10,6 +10,8 @@
     from io import BytesIO
 
 import unittest
+from hypothesis import given
+from hypothesis.strategies import integers, binary, lists, just
 
 global VERBOSE
 VERBOSE=False
@@ -52,12 +54,6 @@ def _help_test_random():
     ss = [ randstr(l//k) for x in range(k) ]
     _h(k, m, ss)
 
-def _help_test_random_with_l(l):
-    m = random.randrange(1, 257)
-    k = random.randrange(1, m+1)
-    ss = [ randstr(l//k) for x in range(k) ]
-    _h(k, m, ss)
-
 def _h_easy(k, m, s):
     encer = zfec.easyfec.Encoder(k, m)
     nums_and_blocks = list(enumerate(encer.encode(s)))
@@ -127,11 +123,35 @@ def test_from_agl_py(self):
         # print "after decoding:"
         # print "b0: %s, b1: %s" % tuple(base64.b16encode(x) for x in [b0, b1])
 
-    def test_small(self):
-        for i in range(16):
-            _help_test_random_with_l(i)
-        if VERBOSE:
-            print("%d randomized tests pass." % (i+1))
+    @given(
+        integers(min_value=0, max_value=15).flatmap(
+            lambda l:
+            integers(min_value=1, max_value=256).flatmap(
+                lambda m:
+                integers(min_value=1, max_value=m).flatmap(
+                    lambda k:
+                    lists(
+                        binary(min_size=l//k, max_size=l//k),
+                        min_size=k,
+                        max_size=k,
+                    ).flatmap(
+                        lambda ss: just((k, m, ss)),
+                    ),
+                ),
+            ),
+        ),
+    )
+    def test_small(self, kmss):
+        """
+        Short primary blocks (length between 0 and 15) round-trip through
+        Encoder / Decoder for all values of k, m, such that:
+
+          * 1 <= m <= 256
+          * 1 <= k <= m
+
+        """
+        (k, m, ss) = kmss
+        _h(k, m, ss)
 
     def test_random(self):
         for i in range(3):