Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#12055 closed enhancement (wontfix)

OFW should sanity-check size of .zd files before starting to write them

Reported by: cjb Owned by: Quozl
Priority: normal Milestone: 13.1.0
Component: build-system Version: not specified
Keywords: Cc: wmb@…, martin.langhoff, dsd
Blocked By: Blocking:
Deployments affected: Action Needed: no action
Verified: no

Description

The failure mode I'm trying to protect against is the one where you only have a partial download of the file, and you only realize it after fs-update spends five minutes flashing and then says "Short read of zdata file" unexpectedly. It would save time (and wondering about why it failed) for it to immediately tell you that there's no way the .zd file you've provided can be complete, instead of waiting until it runs out of data.

We should store the ultimate file size in bytes in a header of the .zd file, and test it against the file size on disk before doing anything else.

Attachments (1)

0001-zhashfs-add-header-data-for-sanity-check-of-file-siz.patch (1.7 KB) - added by Quozl 2 years ago.
olpc-os-builder patch for review

Download all attachments as: .zip

Change History (14)

Changed 2 years ago by Quozl

olpc-os-builder patch for review

comment:1 Changed 2 years ago by Quozl

  • Action Needed changed from never set to review
  • Cc wmb@… martin.langhoff dsd added; wmb removed
  • Component changed from ofw - open firmware to build-system
  • Status changed from new to assigned

Martin, Daniel, olpc-os-builder support for sanity-check of file size requires a patch, see previous change to this ticket. Please review and commit. Let me know when it is in the XO-4 builds.

Open Firmware support added in svn 3204. Will be included in next release.

(Some bugs or filesystem corruptions have a footprint of filesize not matching amount of data read, so solving this ticket will also detect these.)

comment:2 Changed 2 years ago by Quozl

Released in Q3C09 for XO-1.5. Patch to olpc-os-builder is still not merged.

comment:3 Changed 2 years ago by dsd

  • Action Needed changed from review to add to build
  • Milestone changed from Not Triaged to 13.1.0

Thanks, pushed to olpc-os-builder master after a quick test.

comment:4 Changed 2 years ago by dsd

  • Action Needed changed from add to build to test in build

Test in 13.1.0 build 1.

comment:5 Changed 2 years ago by greenfeld

  • Action Needed changed from test in build to no action
  • Resolution set to fixed
  • Status changed from assigned to closed

Installed 13.1.0 os1 using Q3C07 to verify new .zd could be installed by older OFW. After reflashing verified that Q3C09 refused to install a file which was just the first 5 MB of 13.1.0 os1.

The new firmware was used to install 11.3.1 os885 as well as 13.1.0 os1 again to verify compatibility with older .zd files.

Tested on XO-1.5; to the best of my knowledge this is not in Q4D21 for XO-1.75 as the latter will try to install truncated files.

comment:6 Changed 2 years ago by dsd

  • Resolution fixed deleted
  • Status changed from closed to reopened

This change has broken the secure fs-update path from Q3C08 or older. When trying a secure fs-update to install a .zd file including the size line, this error occurs before image flashing starts:

Trying disk:\fs1.zip
Filesystem image found -
<buffer@1c0000>:9: Spec line mismatch

comment:7 Changed 2 years ago by Quozl

  • Resolution set to duplicate
  • Status changed from reopened to closed

Being handled in #12186.

comment:8 Changed 2 years ago by dsd

  • Resolution duplicate deleted
  • Status changed from closed to reopened

I would imagine that #12186 is a different issue: writing to data-buffer 0 is bad, but hasn't caused known failures (OFW was always doing it before r3369). And Q3C09 was already working before the #12186 fix. What needs to be addressed now is compatibility of the size: feature in secure image installs for versions such as Q3C07 and Q3C08 which are deployed in the field.

comment:9 Changed 2 years ago by Quozl

Thanks. Can you provide signed builds to test against please?

comment:10 Changed 2 years ago by Quozl

Never mind, saw the test case instructions in #12186.

comment:11 Changed 2 years ago by Quozl

  • Action Needed changed from no action to code

Reproduced with 31005o4.zd with bundle-suffix set to 4. data-buffer is zero, but as you say harmlessly so. ?compare-spec-line called by zblocks: is failing with "Spec line mismatch" because the two lines are different; one is zblocks: 20000 3bcf, the other is [ifdef] size: size: 00003052d19d [then]. I see no obvious solution, so I recommend you revert the olpc-os-builder patch, and then we can close this ticket as wontfix.

comment:12 Changed 2 years ago by dsd

  • Resolution set to wontfix
  • Status changed from reopened to closed

OK, done. Thanks for investigating. That will take effect for 13.1.0 build 8 and onwards.

comment:13 Changed 2 years ago by Quozl

  • Action Needed changed from code to no action

Is in Q4D25 targeted for 13.2.0.

Note: See TracTickets for help on using tickets.