Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gzStreamUpdater update finishes successfully, but MD5 checksums do not match #68

Open
arkhipenko opened this issue May 31, 2023 · 6 comments

Comments

@arkhipenko
Copy link

arkhipenko commented May 31, 2023

Hi @tobozo
Thank you for this library! I know what it means to maintain an open-source library, so your effort is really appreciated!

I came across an issue that maybe you have some insight into:
I am updating esp32 firmware using your library based on the gzip file delivered to the device over OTA.
The update actually ends up being successful (and the fimrware works!), but the resulting MD5 from the Update class does not match the calculated MD5 of the binary inside the gzip file. Any idea why this might be the case?

Here is the code snippet - nothing fancy - straight from one of your examples.

  Log.notice("OtaDispatcher::finishOta (gz bin) - processing firmware.gz file (full size: %d)" CR, tarGzIO.output_size);

  GZUnpacker->haltOnError( false ); // stop on fail (manual restart/reset required)
  GZUnpacker->setupFSCallbacks( targzTotalBytesFn, targzFreeBytesFn ); // prevent the partition from exploding, recommended
  GZUnpacker->setGzProgressCallback( BaseUnpacker::targzNullProgressCallback ); // targzNullProgressCallback or defaultProgressCallback
  GZUnpacker->setLoggerCallback( BaseUnpacker::targzNullLoggerCallback );    // gz log verbosity

  if( !GZUnpacker->gzStreamUpdater( (Stream *)&mGzFile, tarGzIO.output_size, 0, false ) ) {
      Log.error("OtaDispatcher::finishOta (gz bin) -gzStreamUpdater failed with return code #%d" CR, GZUnpacker->tarGzGetError() );
      _failed();
  }
  else {
      Log.notice("OtaDispatcher::finishOta (gz bin) - OTA Update ended successfully" CR);
      Log.notice("OtaDispatcher::finishOta (gz bin) - Comparing MD5 strings" CR);

      Log.notice("OtaDispatcher::finishOta (gz bin) - provided: %S" CR, mMD5String);
      Log.notice("OtaDispatcher::finishOta (gz bin) - calculated: %S" CR, Update.md5String());

      if ( mMD5String != Update.md5String() ) {
          Log.error("OtaDispatcher::finishOta (gz bin) - MD5 strings do not match" CR);
          _failed();     
      }
  }

I browsed through your code, and you seem to have Update.begin() and Update.write() and Update.end(true) in exactly the same places I would put them, and yet:

00:00:09:23.433 N: OtaDispatcher::finishOta (gz bin) - processing firmware.gz file (full size: 1140960)
00:00:09:41.756 N: OtaDispatcher::finishOta (gz bin) - OTA Update ended successfully
00:00:09:41.758 N: OtaDispatcher::finishOta (gz bin) - Comparing MD5 strings
00:00:09:41.769 N: OtaDispatcher::finishOta (gz bin) - provided: 3cb229da659a4fb476f8ec3bf16c009a
00:00:09:41.770 N: OtaDispatcher::finishOta (gz bin) - calculated: 6f43ff9a6cac85429333ac8f22516515
00:00:09:41.782 E: OtaDispatcher::finishOta (gz bin) - MD5 strings do not match

Any ideas/suggestions would be greatly appreciated!

@arkhipenko
Copy link
Author

I am particularly interested in why you are altering the stream size here:

      if( !Update.begin( ( ( update_size + SPI_FLASH_SEC_SIZE-1 ) & ~( SPI_FLASH_SEC_SIZE-1 ) ), partition ) ) {

@tobozo
Copy link
Owner

tobozo commented May 31, 2023

hi, thanks for your feedback 👍

I'm not sure why tarGzIO.output_size is given as second argument, but does it improve when you use UPDATE_SIZE_UNKNOWN instead?

 GZUnpacker->gzStreamUpdater( (Stream *)&mGzFile, UPDATE_SIZE_UNKNOWN, 0, false );

An ota partition size must be a multiple of 4096, so it may be padded with zeroes depending on the remaining bytes in the last gzip buffer, as a result the effective update size may differ from the binary size.

@arkhipenko
Copy link
Author

Thanks for a quick reply @tobozo !

tarGzIO.output_size is the size of the unzaipped file.
Using UPDATE_SIZE_UNKNOWN did not help - that's where I started.

Are you padding the buffer with 0's in your code?

@arkhipenko
Copy link
Author

The MD5 calculation does not need to be aligned to 4K.

From the Updater class:


in bool UpdateClass::begin();
_md5 = MD5Builder();
...
_md5.begin();
...

in bool UpdateClass::_writeBuffer(): 
_md5.add(_buffer, _bufferLen);
...
in bool UpdateClass::end():
 _md5.calculate();

So the difference might be coming from the fact that all buffer lengths do not add up to the file size
md5.add(_buffer, _bufferLen);

@arkhipenko
Copy link
Author

I also noticed that I am on 1.1.4 and your master is different. Will try that.

@tobozo
Copy link
Owner

tobozo commented May 31, 2023

( ( update_size + SPI_FLASH_SEC_SIZE-1 ) & ~( SPI_FLASH_SEC_SIZE-1 ) ) calculates the destination partition size so that it matches the "multiple of 4096" requirement.

btw the new GzUpdateClass can also do stream update:

https://github.com/tobozo/ESP32-targz/blob/master/src/ESP32-targz-lib.hpp#L311-L383

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants