facebook / openbmc

OpenBMC is an open software framework to build a complete Linux image for a Board Management Controller (BMC).
633 stars 277 forks source link

flashy: Check image file size against flash device size #150

Closed lhl2617 closed 3 years ago

lhl2617 commented 3 years ago

Summary

As title. Corresponds to fw-util in https://github.com/facebook/openbmc/commit/f137c9abc78630971d95c589641e196a11c9584f

The difference is that the image file size is validated against the target flash device's size, if the flash target device ID is provided (it is optional during ./flashy -checkimage or the check-image utility, during which the image file size check is bypassed. This is to support validating images on dev servers,)

Test plan

Unit tests (go test ./...)

facebook-github-bot commented 3 years ago

@kawmarco has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

kawmarco commented 3 years ago

Hey @lhl2617, thanks for the PRs! For this one, I'm wondering if we could compare the image size with the mtd device size instead (imagining we'd eventually have images > 32M eventually), would that make sense?

lhl2617 commented 3 years ago

@kawmarco Good point! One issue is that sometimes we do not always know the target mtd device, but during flashing we always do. I'll modify the function to take in an optional target flash device, which if defined, this function will cross check the size of the image against the size of the flash device.

lhl2617 commented 3 years ago

@kawmarco Good point! One issue is that sometimes we do not always know the target mtd device, but during flashing we always do. I'll modify the function to take in an optional target flash device, which if defined, this function will cross check the size of the image against the size of the flash device.

Updated as per this description :)

facebook-github-bot commented 3 years ago

@kawmarco has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

kawmarco commented 3 years ago

Tested locally, looking good :-)

/tmp/tmp.Gjc0hUHkaK$ cat flash-wedge40 <( dd if=/dev/urandom count=16 bs=1M) > flash-wedge40-big

~/openbmc/tools/flashy$ scripts/run_flashy_remote.sh --device mtd:flash0 --imagepath /tmp/tmp.Gjc0hUHkaK/flash-wedge40-big  --host $target_hostname
...
2021/04/27 07:04:12 {Err:Image file size check failed: Image size (35995230B) larger than flash device size (33554432B)
github.com/facebook/openbmc/tools/flashy/lib/validate/image.glob..func2
    $HOME/openbmc/tools/flashy/lib/validate/image/image.go:65
github.com/facebook/openbmc/tools/flashy/checks_and_remediations/common.validateImagePartitions
    $HOME/openbmc/tools/flashy/checks_and_remediations/common/41_validate_image_partitions.go:46
main.main
    $HOME/openbmc/tools/flashy/flashy.go:139
...
deathowl commented 3 years ago

@lhl2617 merged. Thanks :) 🥇

lhl2617 commented 3 years ago

Thanks @deathowl

image

templeOS