Skip to content

Commit e2c691e

Browse files
authored
Merge pull request #226 from rust-osdev/tag-casting
multiboot2: massive refactoring, removed UB, Miri passes all tests
2 parents c8030ae + 3484e63 commit e2c691e

36 files changed

+1199
-900
lines changed

.github/workflows/_build-rust.yml

+6-3
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,11 @@ jobs:
9898
run: cargo test --verbose
9999
- name: Unit Test with Miri
100100
if: inputs.do-miri
101-
# "--tests" so that the doctests are skipped. Currently, the doctest
102-
# in miri fails.
103101
run: |
104102
rustup component add miri
105-
cargo miri test --tests
103+
# Run with stack-borrow model
104+
# XXX Temporarily, just for multiboot2 crate.
105+
cargo miri test -p multiboot2
106+
# Run with tree-borrow model
107+
# XXX Temporarily, just for multiboot2 crate.
108+
MIRIFLAGS=-Zmiri-tree-borrows cargo +nightly miri test -p multiboot2

.github/workflows/rust.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
name: build (msrv)
2525
uses: ./.github/workflows/_build-rust.yml
2626
with:
27-
rust-version: 1.75.0 # MSRV
27+
rust-version: 1.70.0 # MSRV
2828
do-style-check: false
2929
features: builder
3030

@@ -50,7 +50,7 @@ jobs:
5050
needs: build_msrv
5151
uses: ./.github/workflows/_build-rust.yml
5252
with:
53-
rust-version: 1.75.0 # MSRV
53+
rust-version: 1.70.0 # MSRV
5454
do-style-check: false
5555
rust-target: thumbv7em-none-eabihf
5656
features: builder
@@ -107,7 +107,7 @@ jobs:
107107
needs: build_msrv
108108
uses: ./.github/workflows/_build-rust.yml
109109
with:
110-
rust-version: 1.75.0 # MSRV
110+
rust-version: 1.70.0 # MSRV
111111
do-style-check: true
112112
do-test: false
113113
features: builder

Cargo.lock

+2-18
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ exclude = [
1010

1111
[workspace.dependencies]
1212
bitflags = "2.6.0"
13-
derive_more = { version = "1.0.0", default-features = false, features = ["display"] }
13+
derive_more = { version = "~0.99.18", default-features = false, features = ["display"] }
1414
log = { version = "~0.4", default-features = false }
1515

1616
# This way, the "multiboot2" dependency in the multiboot2-header crate can be

integration-test/bins/Cargo.lock

+2-18
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

integration-test/bins/multiboot2_chainloader/src/loader.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,15 @@ pub fn load_module(mut modules: multiboot::information::ModuleIter) -> ! {
4444

4545
// build MBI
4646
let mbi = multiboot2::builder::InformationBuilder::new()
47-
.bootloader_name_tag(BootLoaderNameTag::new("mb2_integrationtest_chainloader"))
48-
.command_line_tag(CommandLineTag::new("chainloaded YEAH"))
47+
.bootloader_name_tag(&BootLoaderNameTag::new("mb2_integrationtest_chainloader"))
48+
.command_line_tag(&CommandLineTag::new("chainloaded YEAH"))
4949
// random non-sense memory map
50-
.memory_map_tag(MemoryMapTag::new(&[MemoryArea::new(
50+
.memory_map_tag(&MemoryMapTag::new(&[MemoryArea::new(
5151
0,
5252
0xffffffff,
5353
MemoryAreaType::Reserved,
5454
)]))
55-
.add_module_tag(ModuleTag::new(
55+
.add_module_tag(&ModuleTag::new(
5656
elf_mod.start as u32,
5757
elf_mod.end as u32,
5858
elf_mod.string.unwrap(),

multiboot2-header/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ readme = "README.md"
2626
homepage = "https://github.com/rust-osdev/multiboot2-header"
2727
repository = "https://github.com/rust-osdev/multiboot2"
2828
documentation = "https://docs.rs/multiboot2-header"
29-
rust-version = "1.75"
29+
rust-version = "1.70"
3030

3131
[[example]]
3232
name = "minimal"

multiboot2-header/Changelog.md

-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
## Unreleased
44

55
- **Breaking** All functions that returns something useful are now `#[must_use]`
6-
- updated dependencies
7-
- documentation enhancements
86

97
## 0.4.0 (2024-05-01)
108

multiboot2-header/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ bytes of the ELF. See Multiboot2 specification.
7777

7878
## MSRV
7979

80-
The MSRV is 1.75.0 stable.
80+
The MSRV is 1.70.0 stable.
8181

8282
## License & Contribution
8383

multiboot2-header/src/builder/header.rs

+1
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ impl HeaderBuilder {
255255
/// Adds information requests from the
256256
/// [`InformationRequestHeaderTagBuilder`] to the builder.
257257
#[must_use]
258+
#[allow(clippy::missing_const_for_fn)] // only in Rust 1.70 necessary
258259
pub fn information_request_tag(
259260
mut self,
260261
information_request_tag: InformationRequestHeaderTagBuilder,

multiboot2-header/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
//!
3535
//! ## MSRV
3636
//!
37-
//! The MSRV is 1.75.0 stable.
37+
//! The MSRV is 1.70.0 stable.
3838
3939
#![no_std]
4040
#![cfg_attr(feature = "unstable", feature(error_in_core))]

multiboot2/Cargo.toml

+4-2
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ readme = "README.md"
3131
homepage = "https://github.com/rust-osdev/multiboot2"
3232
repository = "https://github.com/rust-osdev/multiboot2"
3333
documentation = "https://docs.rs/multiboot2"
34-
rust-version = "1.75"
34+
rust-version = "1.70"
3535

3636
[features]
3737
default = ["builder"]
@@ -45,12 +45,14 @@ bitflags.workspace = true
4545
derive_more.workspace = true
4646
log.workspace = true
4747

48+
ptr_meta = { version = "~0.2", default-features = false }
4849
# We only use a very basic type definition from this crate. To prevent MSRV
4950
# bumps from uefi-raw, I restrict this here. Upstream users are likely to have
5051
# two versions of this library in it, which is no problem, as we only use the
5152
# type definition.
5253
uefi-raw = { version = "~0.5", default-features = false }
53-
ptr_meta = { version = "~0.2", default-features = false }
54+
55+
[dev-dependencies]
5456

5557
[package.metadata.docs.rs]
5658
all-features = true

multiboot2/Changelog.md

+30-6
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,38 @@
22

33
## Unreleased
44

5-
- **Breaking** All functions that returns something useful are now `#[must_use]`
6-
- **Breaking** More public fields in tags were replaced by public getters, such
5+
-
6+
7+
## 0.21.0 (2024-08-17)
8+
9+
This release contains a massive refactoring of various internals. Now, **all
10+
unit tests pass Miri**, thus we removed lots of undefined behaviour and
11+
increased the memory safety! 🎉 Only a small part of these internal refactorings
12+
leak to the public interface. If you don't provide external custom tags, you
13+
should be fine.
14+
15+
Please note that **all previous releases** must be considered unsafe, as they
16+
contain UB. However, it is never clear how UB results in immediate incorrect
17+
behaviour and it _might_ work. **Nevertheless, please migrate to the latest
18+
release and you'll be fine!**
19+
20+
All previous releases on crates.io have been yanked.
21+
22+
- **Breaking:** All functions that returns something useful are
23+
now `#[must_use]`
24+
- **Breaking:** More public fields in tags were replaced by public getters, such
725
as `SmbiosTag::major()`
8-
- updated dependencies
9-
- MSRV is 1.75
26+
- **Breaking:** Methods of `InformationBuilder` to add tags now consume
27+
references instead of owned values
28+
- **Breaking:** The `BoxedDst` has been removed in favor of a normal Rust `Box`.
29+
This only affects you if you use the `builder` feature.
30+
- **Breaking:** MSRV is 1.75
31+
- **Breaking:** Introduced new `TagHeader` type as replacement for the `Tag`
32+
type that will be changed in the next step. `Tag` has been renamed to an
33+
internal-only `GenericTag` type.
34+
- Added missing `InformationBuilder::vbe_info_tag`
1035
- documentation enhancements
11-
- Introduced new `TagHeader` type as replacement for the `Tag` type that will
12-
be changed in the next step.
36+
- updated dependencies
1337

1438
## 0.20.2 (2024-05-26)
1539

multiboot2/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ tag_, which is a tag of type `0` and size `8`.
4545

4646
## MSRV
4747

48-
The MSRV is 1.75.0 stable.
48+
The MSRV is 1.70.0 stable.
4949

5050
## License & Contribution
5151

multiboot2/src/boot_information.rs

+19-15
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@ use derive_more::Display;
2121
pub enum MbiLoadError {
2222
/// The address is invalid. Make sure that the address is 8-byte aligned,
2323
/// according to the spec.
24-
#[display("The address is invalid")]
24+
#[display(fmt = "The address is invalid")]
2525
IllegalAddress,
2626
/// The total size of the multiboot2 information structure must be not zero
2727
/// and a multiple of 8.
28-
#[display("The size of the MBI is unexpected")]
28+
#[display(fmt = "The size of the MBI is unexpected")]
2929
IllegalTotalSize(u32),
3030
/// Missing end tag. Each multiboot2 boot information requires to have an
3131
/// end tag.
32-
#[display("There is no end tag")]
32+
#[display(fmt = "There is no end tag")]
3333
NoEndTag,
3434
}
3535

@@ -38,7 +38,7 @@ impl core::error::Error for MbiLoadError {}
3838

3939
/// The basic header of a [`BootInformation`] as sized Rust type.
4040
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
41-
#[repr(C)]
41+
#[repr(C, align(8))]
4242
pub struct BootInformationHeader {
4343
// size is multiple of 8
4444
total_size: u32,
@@ -68,7 +68,7 @@ impl AsBytes for BootInformationHeader {}
6868
/// This type holds the whole data of the MBI. This helps to better satisfy miri
6969
/// when it checks for memory issues.
7070
#[derive(ptr_meta::Pointee)]
71-
#[repr(C)]
71+
#[repr(C, align(8))]
7272
struct BootInformationInner {
7373
header: BootInformationHeader,
7474
tags: [u8],
@@ -221,6 +221,8 @@ impl<'a> BootInformation<'a> {
221221
/// Otherwise, if the [`TagType::EfiBs`] tag is present, this returns `None`
222222
/// as it is strictly recommended to get the memory map from the `uefi`
223223
/// services.
224+
///
225+
/// [`TagType::EfiBs`]: crate::TagType::EfiBs
224226
#[must_use]
225227
pub fn efi_memory_map_tag(&self) -> Option<&EFIMemoryMapTag> {
226228
// If the EFIBootServicesNotExited is present, then we should not use
@@ -277,8 +279,8 @@ impl<'a> BootInformation<'a> {
277279
pub fn elf_sections(&self) -> Option<ElfSectionIter> {
278280
let tag = self.get_tag::<ElfSectionsTag>();
279281
tag.map(|t| {
280-
assert!((t.entry_size * t.shndx) <= t.size);
281-
t.sections()
282+
assert!((t.entry_size() * t.shndx()) <= t.size() as u32);
283+
t.sections_iter()
282284
})
283285
}
284286

@@ -359,7 +361,7 @@ impl<'a> BootInformation<'a> {
359361
/// special handling is required. This is reflected by code-comments.
360362
///
361363
/// ```no_run
362-
/// use multiboot2::{BootInformation, BootInformationHeader, StringError, Tag, TagTrait, TagType, TagTypeId};
364+
/// use multiboot2::{BootInformation, BootInformationHeader, parse_slice_as_string, StringError, TagHeader, TagTrait, TagType, TagTypeId};
363365
///
364366
/// #[repr(C)]
365367
/// #[derive(multiboot2::Pointee)] // Only needed for DSTs.
@@ -374,17 +376,17 @@ impl<'a> BootInformation<'a> {
374376
/// impl TagTrait for CustomTag {
375377
/// const ID: TagType = TagType::Custom(0x1337);
376378
///
377-
/// fn dst_size(base_tag: &Tag) -> usize {
379+
/// fn dst_len(header: &TagHeader) -> usize {
378380
/// // The size of the sized portion of the custom tag.
379381
/// let tag_base_size = 8; // id + size is 8 byte in size
380-
/// assert!(base_tag.size >= 8);
381-
/// base_tag.size as usize - tag_base_size
382+
/// assert!(header.size >= 8);
383+
/// header.size as usize - tag_base_size
382384
/// }
383385
/// }
384386
///
385387
/// impl CustomTag {
386388
/// fn name(&self) -> Result<&str, StringError> {
387-
/// Tag::parse_slice_as_string(&self.name)
389+
/// parse_slice_as_string(&self.name)
388390
/// }
389391
/// }
390392
/// let mbi_ptr = 0xdeadbeef as *const BootInformationHeader;
@@ -395,11 +397,13 @@ impl<'a> BootInformation<'a> {
395397
/// .unwrap();
396398
/// assert_eq!(tag.name(), Ok("name"));
397399
/// ```
400+
///
401+
/// [`TagType`]: crate::TagType
398402
#[must_use]
399403
pub fn get_tag<TagT: TagTrait + ?Sized + 'a>(&'a self) -> Option<&'a TagT> {
400404
self.tags()
401-
.find(|tag| tag.typ == TagT::ID)
402-
.map(|tag| tag.cast_tag::<TagT>())
405+
.find(|tag| tag.header().typ == TagT::ID)
406+
.map(|tag| tag.cast::<TagT>())
403407
}
404408

405409
/// Returns an iterator over all tags.
@@ -440,7 +444,7 @@ impl fmt::Debug for BootInformation<'_> {
440444
if elf_sections_tag_entries_count > ELF_SECTIONS_LIMIT {
441445
debug.field("elf_sections (count)", &elf_sections_tag_entries_count);
442446
} else {
443-
debug.field("elf_sections", &self.elf_sections().unwrap_or_default());
447+
debug.field("elf_sections", &self.elf_sections());
444448
}
445449
}
446450

0 commit comments

Comments
 (0)