Skip to content

Commit 0f63603

Browse files
Johan-Liebert1cgwalters
authored andcommitted
erofs: Fix reading directory entries
Read inline inode entries only if not empty Also update the API to return `None` if the inode has no inline data Before this patch, gathering objects from EROFS, the program panics with the following error ``` thread 'main' panicked at crates/composefs/src/erofs/reader.rs:404:13: range end index 12 out of range for slice of length 0 ``` Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> Signed-off-by: Colin Walters <walters@verbum.org>
1 parent e74ae21 commit 0f63603

File tree

2 files changed

+34
-30
lines changed

2 files changed

+34
-30
lines changed

crates/composefs/src/erofs/debug.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,8 @@ impl<'img> ImageVisitor<'img> {
174174
}
175175

176176
if inode.mode().is_dir() {
177-
let inline = inode.inline();
178-
if !inline.is_empty() {
179-
let inline_block = DirectoryBlock::ref_from_bytes(inode.inline()).unwrap();
177+
if let Some(inline) = inode.inline() {
178+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
180179
self.visit_directory_block(inline_block, path);
181180
}
182181

@@ -314,12 +313,10 @@ impl<T: fmt::Debug + InodeHeader> fmt::Debug for Inode<T> {
314313
// - directory data: dump the entries
315314
// - small inline text string: print it
316315
// - otherwise, hexdump
317-
let inline = self.inline();
318-
319-
// No inline data
320-
if inline.is_empty() {
316+
let Some(inline) = self.inline() else {
317+
// No inline data
321318
return Ok(());
322-
}
319+
};
323320

324321
// Directory dump
325322
if self.header.mode().is_dir() {

crates/composefs/src/erofs/reader.rs

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ pub trait InodeOps {
167167
/// Returns the extended attributes section if present
168168
fn xattrs(&self) -> Option<&InodeXAttrs>;
169169
/// Returns the inline data portion
170-
fn inline(&self) -> &[u8];
170+
fn inline(&self) -> Option<&[u8]>;
171171
/// Returns the range of block IDs used by this inode
172172
fn blocks(&self, blkszbits: u8) -> Range<u64>;
173173
}
@@ -202,8 +202,14 @@ impl<Header: InodeHeader> InodeOps for &Inode<Header> {
202202
}
203203
}
204204

205-
fn inline(&self) -> &[u8] {
206-
&self.data[self.header.xattr_size()..]
205+
fn inline(&self) -> Option<&[u8]> {
206+
let data = &self.data[self.header.xattr_size()..];
207+
208+
if data.is_empty() {
209+
return None;
210+
}
211+
212+
Some(data)
207213
}
208214

209215
fn blocks(&self, blkszbits: u8) -> Range<u64> {
@@ -281,7 +287,7 @@ impl InodeOps for InodeType<'_> {
281287
}
282288
}
283289

284-
fn inline(&self) -> &[u8] {
290+
fn inline(&self) -> Option<&[u8]> {
285291
match self {
286292
Self::Compact(inode) => inode.inline(),
287293
Self::Extended(inode) => inode.inline(),
@@ -615,8 +621,10 @@ impl<ObjectID: FsVerityHashValue> ObjectCollector<ObjectID> {
615621
self.visit_directory_block(img.directory_block(blkid));
616622
}
617623

618-
let tail = DirectoryBlock::ref_from_bytes(inode.inline()).unwrap();
619-
self.visit_directory_block(tail);
624+
if let Some(inline) = inode.inline() {
625+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
626+
self.visit_directory_block(inline_block);
627+
}
620628
}
621629

622630
Ok(())
@@ -662,8 +670,8 @@ mod tests {
662670
let mut found_names = Vec::new();
663671

664672
// Read inline entries if present
665-
if !inode.inline().is_empty() {
666-
let inline_block = DirectoryBlock::ref_from_bytes(inode.inline()).unwrap();
673+
if let Some(inline) = inode.inline() {
674+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
667675
for entry in inline_block.entries() {
668676
let name = std::str::from_utf8(entry.name).unwrap();
669677
found_names.push(name.to_string());
@@ -709,8 +717,8 @@ mod tests {
709717
// Find empty_dir entry
710718
let root_inode = img.root();
711719
let mut empty_dir_nid = None;
712-
if !root_inode.inline().is_empty() {
713-
let inline_block = DirectoryBlock::ref_from_bytes(root_inode.inline()).unwrap();
720+
if let Some(inline) = root_inode.inline() {
721+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
714722
for entry in inline_block.entries() {
715723
if entry.name == b"empty_dir" {
716724
empty_dir_nid = Some(entry.nid());
@@ -748,8 +756,8 @@ mod tests {
748756
// Find dir1
749757
let root_inode = img.root();
750758
let mut dir1_nid = None;
751-
if !root_inode.inline().is_empty() {
752-
let inline_block = DirectoryBlock::ref_from_bytes(root_inode.inline()).unwrap();
759+
if let Some(inline) = root_inode.inline() {
760+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
753761
for entry in inline_block.entries() {
754762
if entry.name == b"dir1" {
755763
dir1_nid = Some(entry.nid());
@@ -792,8 +800,8 @@ mod tests {
792800
// Find bigdir
793801
let root_inode = img.root();
794802
let mut bigdir_nid = None;
795-
if !root_inode.inline().is_empty() {
796-
let inline_block = DirectoryBlock::ref_from_bytes(root_inode.inline()).unwrap();
803+
if let Some(inline) = root_inode.inline() {
804+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
797805
for entry in inline_block.entries() {
798806
if entry.name == b"bigdir" {
799807
bigdir_nid = Some(entry.nid());
@@ -845,8 +853,8 @@ mod tests {
845853
let find_entry = |parent_nid: u64, name: &[u8]| -> u64 {
846854
let inode = img.inode(parent_nid);
847855

848-
if !inode.inline().is_empty() {
849-
let inline_block = DirectoryBlock::ref_from_bytes(inode.inline()).unwrap();
856+
if let Some(inline) = inode.inline() {
857+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
850858
for entry in inline_block.entries() {
851859
if entry.name == name {
852860
return entry.nid();
@@ -892,8 +900,8 @@ mod tests {
892900

893901
let root_inode = img.root();
894902
let mut mixed_nid = None;
895-
if !root_inode.inline().is_empty() {
896-
let inline_block = DirectoryBlock::ref_from_bytes(root_inode.inline()).unwrap();
903+
if let Some(inline) = root_inode.inline() {
904+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
897905
for entry in inline_block.entries() {
898906
if entry.name == b"mixed" {
899907
mixed_nid = Some(entry.nid());
@@ -943,7 +951,6 @@ mod tests {
943951
}
944952

945953
#[test]
946-
#[ignore = "Needs https://github.com/containers/composefs-rs/pull/188"]
947954
fn test_pr188_empty_inline_directory() -> anyhow::Result<()> {
948955
// Regression test for https://github.com/containers/composefs-rs/pull/188
949956
//
@@ -1019,8 +1026,8 @@ mod tests {
10191026
let mut entries_map: HashMap<Vec<u8>, u64> = HashMap::new();
10201027
let root_inode = img.root();
10211028

1022-
if !root_inode.inline().is_empty() {
1023-
let inline_block = DirectoryBlock::ref_from_bytes(root_inode.inline()).unwrap();
1029+
if let Some(inline) = root_inode.inline() {
1030+
let inline_block = DirectoryBlock::ref_from_bytes(inline).unwrap();
10241031
for entry in inline_block.entries() {
10251032
entries_map.insert(entry.name.to_vec(), entry.nid());
10261033
}
@@ -1042,6 +1049,6 @@ mod tests {
10421049
assert_eq!(file1_inode.size(), 5);
10431050

10441051
let inline_data = file1_inode.inline();
1045-
assert_eq!(inline_data, b"hello");
1052+
assert_eq!(inline_data, Some(b"hello".as_slice()));
10461053
}
10471054
}

0 commit comments

Comments
 (0)