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

Standardize LCOW uVM bootfiles update #1861

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

helsaawy
Copy link
Contributor

NewDefaultOptionsLCOW sets RootFSFile and KernelFile depending on the contents of the (default) BootFilesPath directory and KerenelDirect field.
However, if BootFilesPath is subsequently updated, those fields are not updated.
This can result in inconsistent behavior, where (depending on if the default BootFilesPath contains vmlinux and rootfs.vhd files), a uVM created with an overridden BootFilesPath may either use initrd (kernel) or vmlinux (rootfs.vhd), respectively.

Add a UpdateBootFilesPath function to consistently change the BootFilesPath and associated options.
Update annotation handling to use UpdateBootFilesPath. Security policy is still performed after the update, so settings will be re-overridden for the confidential case, or by other annotations, so existing (normal) behavior is persisted.

@helsaawy helsaawy requested a review from a team as a code owner July 31, 2023 19:11
@katiewasnothere katiewasnothere self-assigned this Aug 11, 2023
`NewDefaultOptionsLCOW` sets `RootFSFile` and `KernelFile` depending
on the contents of the (default) `BootFilesPath` directory and
`KerenelDirect` field.
However, if `BootFilesPath` is subsequently updated, those fields are
not updated.
This can result in inconsistent behavior, where (depending on if the
default `BootFilesPath` contains `vmlinux` and `rootfs.vhd` files), a
uVM created with an overridden `BootFilesPath` may either use `initrd`
(`kernel`) or `vmlinux` (`rootfs.vhd`), respectively.

Add a `UpdateBootFilesPath` function to consistently change the
`BootFilesPath` and associated options.
Update annotation handling to use `UpdateBootFilesPath`.
Security policy is still performed after the update, so settings will be
re-overridden for the confidential case, or by other annotations, so
existing (normal) behavior is persisted.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Copy link
Contributor

@msscotb msscotb left a comment

Choose a reason for hiding this comment

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

lgtm

@helsaawy helsaawy merged commit ab22a61 into microsoft:main Oct 30, 2023
15 of 16 checks passed
@helsaawy helsaawy deleted the uvm-boot-files branch October 30, 2023 15:23
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
`NewDefaultOptionsLCOW` sets `RootFSFile` and `KernelFile` depending
on the contents of the (default) `BootFilesPath` directory and
`KerenelDirect` field.
However, if `BootFilesPath` is subsequently updated, those fields are
not updated.
This can result in inconsistent behavior, where (depending on if the
default `BootFilesPath` contains `vmlinux` and `rootfs.vhd` files), a
uVM created with an overridden `BootFilesPath` may either use `initrd`
(`kernel`) or `vmlinux` (`rootfs.vhd`), respectively.

Add a `UpdateBootFilesPath` function to consistently change the
`BootFilesPath` and associated options.
Update annotation handling to use `UpdateBootFilesPath`.
Security policy is still performed after the update, so settings will be
re-overridden for the confidential case, or by other annotations, so
existing (normal) behavior is persisted.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
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

Successfully merging this pull request may close these issues.

3 participants