[PATCH 1/2] mceliece6688128f: fix stack overflow crash on win64/wine
Jussi Kivilinna
jussi.kivilinna at iki.fi
Sat Sep 27 08:16:07 CEST 2025
Hello,
On 27/09/2025 02:19, Jacob Bachmeyer wrote:
> On 9/26/25 01:53, Jussi Kivilinna wrote:
>> * cipher/mceliece6688128f.c (pk_gen): Remove 'mat' array allocation and
>> rename function to ...
>> (pk_gen_mat): ... this.
>> (pk_gen): New warper for 'pk_gen_mat' with dynamic allocation of 'mat' array.
>
> Typo? Did you mean "wrapper"?
>
Yes, typo.
> Why do it this way instead of just adding xmalloc()/xfree() calls to pk_gen()? Also, should the buffer be zeroed before releasing it?
>
I wanted to avoid diverging too much from the original, to avoid having to make greater changes to original pk_gen() function. With allocation in pk_gen(), 'mat' would change from 2d array to, for example, simple pointer and then all 'mat[][]' accesses would need to be changed. Or 'mat' could be changed to 'struct { uint64_t arr[][] } *mat' and change accesses to 'mat->arr[][]'.
That buffer probably should be zeroed after although it was not being cleared. I'll add that for v2 of patch. About memory zeroing in general... I see that PQ algorithms use quite a lot of stack arrays in their implementations (some quite large allocations too) but there is no stack memory wiping in place. Maybe there should be? I guess simplest way to wipe used stack would be to add appropriately sized _gcry_burn_stack() calls after each PQ function call, for example in 'kem.c'.
-Jussi
More information about the Gcrypt-devel
mailing list