Re: [PATCH 1/2] mceliece6688128f: fix stack overflow crash on win64/wine

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


_______________________________________________
Gcrypt-devel mailing list
[email protected]
https://lists.gnupg.org/mailman/listinfo/gcrypt-devel