Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

lhl2617
Copy link
Contributor

@lhl2617 lhl2617 commented Apr 19, 2021 •
edited
Loading

Summary

As title. Corresponds to fw-util in f137c9a

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
Copy link
Contributor

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

@kawmarco
Copy link
Contributor

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
Copy link
Contributor Author

lhl2617 commented Apr 26, 2021 •
edited
Loading

@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 lhl2617 marked this pull request as draft April 26, 2021 14:41
@lhl2617 lhl2617 force-pushed the image-file-size-check branch 4 times, most recently from b12eb5b to 1c3f837 Compare April 26, 2021 22:04
@lhl2617 lhl2617 changed the title flashy: Fail image validation when image size is >32MB flashy: Check image file size against flash device size Apr 26, 2021
@lhl2617 lhl2617 force-pushed the image-file-size-check branch 3 times, most recently from d0aa5f8 to ef3460a Compare April 26, 2021 22:41
@lhl2617 lhl2617 marked this pull request as ready for review April 26, 2021 22:43
@lhl2617
Copy link
Contributor Author

lhl2617 commented Apr 26, 2021

@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 :)

@lhl2617 lhl2617 force-pushed the image-file-size-check branch from ef3460a to 82f8710 Compare April 26, 2021 22:56
lhl2617
lhl2617 commented Apr 26, 2021
View reviewed changes
tools/flashy/lib/validate/image/image.go
// Takes in maybeDeviceI denoting a deviceID, which if non-empty is used to
// get the flash device and ensure that the image file size is smaller than the size of the
// flash device.
var ValidateImageFile = func(imageFilePath string, maybeDeviceID string) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved from validate.ValidateImageFile.

@facebook-github-bot
Copy link
Contributor

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

@lhl2617 lhl2617 mentioned this pull request Apr 27, 2021
Closed
@kawmarco
Copy link
Contributor

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
Copy link
Member

@lhl2617 merged. Thanks :) 🥇

facebook-github-bot pushed a commit that referenced this pull request Apr 28, 2021
@lhl2617 @facebook-github-bot
flashy: Check image file size against flash device size (#150)
f4e9048 
Summary:
As title. Corresponds to `fw-util` in f137c9a

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,)

Pull Request resolved: #150

Test Plan: Unit tests (`go test ./...`)

Reviewed By: deathowl

Pulled By: kawmarco

fbshipit-source-id: 6e38204e94
@lhl2617
Copy link
Contributor Author

lhl2617 commented Apr 28, 2021 •
edited
Loading

Thanks @deathowl

image

#templeOS

@lhl2617 lhl2617 deleted the image-file-size-check branch May 10, 2021 15:38
zzzoie pushed a commit to delta-sw103/openbmc that referenced this pull request Jun 8, 2021
@lhl2617 @zzzoie
flashy: Check image file size against flash device size (facebook#150)
cb70ff6 
Summary:
As title. Corresponds to `fw-util` in facebook@f137c9a

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,)

Pull Request resolved: facebook#150

Test Plan: Unit tests (`go test ./...`)

Reviewed By: deathowl

Pulled By: kawmarco

fbshipit-source-id: 6e38204e94
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lhl2617 @facebook-github-bot @kawmarco @deathowl