Skip to content

fv_read_section_efiapi: adjust data pointer - #1747

Merged
makubacki merged 2 commits into
OpenDevicePartnership:mainfrom
olegilyasov:olegilyasov/fv_ReadSection_ptrmiss
Aug 19, 2026
Merged

fv_read_section_efiapi: adjust data pointer#1747
makubacki merged 2 commits into
OpenDevicePartnership:mainfrom
olegilyasov:olegilyasov/fv_ReadSection_ptrmiss

Conversation

@olegilyasov

@olegilyasov olegilyasov commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Description

The fix changes fv_read_section_efiapi so FREEFORM_SUBTYPE_GUID returns the subtype GUID header bytes followed by the payload, instead of returning only the payload.

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

Added test_fv_read_section_freeform_subtype_returns_guid_plus_payload

Integration Instructions

N/A

@patina-automation

patina-automation Bot commented Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

✅ QEMU Validation Passed

All QEMU validation jobs completed successfully.

Note: Q35 is only built on Windows hosts (QEMU boot is disabled due to a QEMU vfat issue).

Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/32204217293

Boot Time to EFI Shell

Platform Elapsed
ArmVirt (Linux Host) 24.5s
Q35 (Linux Host) 26.6s

Dependencies

Repository Ref
patina 52d448c
patina-dxe-core-qemu 5c12f1a
patina-fw-patcher d06ba7e
patina-qemu firmware v4.0.5
patina-qemu build script c1946d7

This comment was automatically generated by the Patina QEMU PR Validation Post workflow.

@github-actions github-actions Bot added the impact:testing Affects testing label Aug 15, 2026
@olegilyasov olegilyasov mentioned this pull request Aug 15, 2026
Closed
1 task
@os-d
os-d requested a lite review from Copilot August 15, 2026 03:39
@os-d
os-d requested review from apop5, joschock, makubacki and os-d and removed request for Copilot August 15, 2026 03:39
@os-d

os-d commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

I accidentally clicked the copilot review button and now I'm not sure how to get it to stop, my apologies.

@olegilyasov
olegilyasov force-pushed the olegilyasov/fv_ReadSection_ptrmiss branch 2 times, most recently from fe73b13 to ddde1a0 Compare August 17, 2026 13:25
@makubacki makubacki linked an issue Aug 17, 2026 that may be closed by this pull request
[Bug]: fv_read_section_efiapi returns wrong data when called for EFI_SECTION_FREEFORM_SUBTYPE_GUID (0x18) section #1746
Closed
1 task
joschock
joschock requested changes Aug 17, 2026
View reviewed changes
Comment thread patina_dxe_core/src/pi_dispatcher/fv.rs Outdated
makubacki
makubacki reviewed Aug 17, 2026
View reviewed changes
Comment thread patina_dxe_core/src/pi_dispatcher/fv.rs Outdated
@olegilyasov
olegilyasov force-pushed the olegilyasov/fv_ReadSection_ptrmiss branch from 809618a to 0a68363 Compare August 18, 2026 21:05
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@olegilyasov @makubacki
fv_read_section_efiapi: adjust data pointer
1cbb395 
- when FV section type EFI_SECTION_FREEFORM_SUBTYPE_GUID
  then the section content includes:
  1) Section Header (4 Bytes)
  2) Subtype GUID (16 Bytes)
  3) Data payload (N Bytes)

- read section function currently returns data pointer as
  section_start + sizeof(section_header) + sizeof(guid)

- the correct data pointer would be
  section_start + sizeof(section_header)

Signed-off-by: Oleg Ilyasov <olegi@ami.com>
@olegilyasov @makubacki
fv_read_section_efiapi fix
52d448c 
- Fix the data pointer for several FV section types:
  Compression, GuidDefined, Version, FreeFormSubtypeGuid.

- Add helper type_specific_header_bytes function onto the SectionHeader type for:
  1) serialization - type-specific header is inserted between common header and the data
  2) PI compliance - ReadSection should return type-specific header + content

- Test: replace hardcoded data with size_of GUID

Signed-off-by: Oleg Ilyasov <olegi@ami.com>
@makubacki
makubacki force-pushed the olegilyasov/fv_ReadSection_ptrmiss branch from 0a68363 to 52d448c Compare August 19, 2026 00:59
@makubacki makubacki added the type:bug Something isn't working label Aug 19, 2026
@makubacki

makubacki commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@olegilyasov, are you okay if this PR is squashed as a single commit? We can merge either way, but if you'd like separate commits, they'll need to be refactored so later commits are additive, and the changes are more logically grouped.

@olegilyasov

olegilyasov commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@olegilyasov, are you okay if this PR is squashed as a single commit? We can merge either way, but if you'd like separate commits, they'll need to be refactored so later commits are additive, and the changes are more logically grouped.

Single commit is totally fine, thank you.

@makubacki
makubacki merged commit 7e37c63 into OpenDevicePartnership:main Aug 19, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@makubacki makubacki makubacki approved these changes
@joschock joschock joschock approved these changes
@os-d os-d Awaiting requested review from os-d
@apop5 apop5 Awaiting requested review from apop5

Assignees

No one assigned

Labels

impact:testing Affects testing type:bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: fv_read_section_efiapi returns wrong data when called for EFI_SECTION_FREEFORM_SUBTYPE_GUID (0x18) section

4 participants

@olegilyasov @os-d @makubacki @joschock