Memset a UDIF header to ensure archive reproducibility. #5
Conversation
|
I have tested the current HEAD of mozilla: it is not deterministic. The patch I propose fixes the problems. |
There was a problem hiding this comment.
(Apologies for the very long delay...this repo is not actively watched.)
This seems pretty sensible to do. For the dmg build case things end up bit for bit identical with and without this patch. I can't seem to make dmg convert work with any ISO I've made so I can't verify that...but having zeros instead of random bits seems like an improvement.
dmg/dmglib.c
Outdated
| @@ -108,7 +108,8 @@ int buildDmg(AbstractFile* abstractIn, AbstractFile* abstractOut, unsigned int B | |||
| ChecksumToken dataForkToken; | |||
|
|
|||
| UDIFResourceFile koly; | |||
|
|
|||
| memset(&koly, 0, sizeof(koly)); | |||
There was a problem hiding this comment.
You don't need to use memset. You can do UDIFResourceFile koly = {0};
Some of the struct padding and fields contained unitialized memory, which caused two successive invocations to produce archives that differed in some bytes.
bhearsum
force-pushed
the
fix-reproducibility
branch
from
fa3b97e
to
2262847
Compare
No worries, thank you for getting to it, we're still interested in upstreaming this patch 😄. |
|
Sorry for delay: yes, we should merge this :) |
For Tor Browser, we have used the following patch for many years (but with another fork).
I am not sure it is needed also with Mozilla's fork (I see that fb0da70 is already about making
dmgdeterministic), butmemsetting the whole struct anyway seems a good idea to me.The original author of the patch is Mike Perry.