[pbs-devel] [PATCH v3 pxar 5/6] accessor: add payload checks for split archives

[Christian Ebner <c.ebner@proxmox.com>]

Add checks for split variant inputs when accessing the payload
contents via the accessor instance. Both cases, accessing via the
safe `contents` method and via the unsafe `contents_at_range` call
are covered.

To be able to perform the checks, the current range is wrapped into a
`ContentRange` type, which also passed the payload offset and size
as expected from the metadata archive. The corresponding interfaces
have been adapted accordingly.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 2:
- moved cargo fmt hunk to own patch

 src/accessor/aio.rs  | 15 ++++++++-----
 src/accessor/mod.rs  | 53 ++++++++++++++++++++++++++++++++++++--------
 src/accessor/sync.rs | 16 +++++++------
 3 files changed, 62 insertions(+), 22 deletions(-)

diff --git a/src/accessor/aio.rs b/src/accessor/aio.rs
index 73b1025..c86adc5 100644
--- a/src/accessor/aio.rs
+++ b/src/accessor/aio.rs
@@ -7,7 +7,6 @@
 use std::future::Future;
 use std::io;
 use std::mem;
-use std::ops::Range;
 use std::os::unix::fs::FileExt;
 use std::path::Path;
 use std::pin::Pin;
@@ -16,6 +15,7 @@ use std::task::{Context, Poll};
 
 use crate::accessor::{self, cache::Cache, MaybeReady, ReadAt, ReadAtOperation};
 use crate::decoder::aio::Decoder;
+use crate::format::ContentRange;
 use crate::format::GoodbyeItem;
 use crate::util;
 use crate::{Entry, PxarVariant};
@@ -153,13 +153,16 @@ impl<T: Clone + ReadAt> Accessor<T> {
     ///
     /// This will provide a reader over an arbitrary range of the archive file, so unless this
     /// comes from a actual file entry data, the contents might not make much sense.
-    pub unsafe fn open_contents_at_range(&self, range: Range<u64>) -> FileContents<T> {
-        FileContents {
-            inner: unsafe { self.inner.open_contents_at_range(range) },
+    pub async unsafe fn open_contents_at_range(
+        &self,
+        range: &ContentRange,
+    ) -> io::Result<FileContents<T>> {
+        Ok(FileContents {
+            inner: unsafe { self.inner.open_contents_at_range(range).await? },
             at: 0,
             buffer: Vec::new(),
             future: None,
-        }
+        })
     }
 
     /// Following a hardlink.
@@ -235,7 +238,7 @@ impl<T: Clone + ReadAt> FileEntry<T> {
     }
 
     /// For use with unsafe accessor methods.
-    pub fn content_range(&self) -> io::Result<Option<Range<u64>>> {
+    pub fn content_range(&self) -> io::Result<Option<ContentRange>> {
         self.inner.content_range()
     }
 
diff --git a/src/accessor/mod.rs b/src/accessor/mod.rs
index 92d689d..9f27dfd 100644
--- a/src/accessor/mod.rs
+++ b/src/accessor/mod.rs
@@ -17,7 +17,7 @@ use endian_trait::Endian;
 
 use crate::binary_tree_array;
 use crate::decoder::{self, DecoderImpl};
-use crate::format::{self, FormatVersion, GoodbyeItem};
+use crate::format::{self, ContentRange, FormatVersion, GoodbyeItem, PayloadRef};
 use crate::util;
 use crate::{Entry, EntryKind, PxarVariant};
 
@@ -336,11 +336,25 @@ impl<T: Clone + ReadAt> AccessorImpl<T> {
     }
 
     /// Allow opening arbitrary contents from a specific range.
-    pub unsafe fn open_contents_at_range(&self, range: Range<u64>) -> FileContentsImpl<T> {
+    pub async unsafe fn open_contents_at_range(
+        &self,
+        range: &format::ContentRange,
+    ) -> io::Result<FileContentsImpl<T>> {
         if let Some((payload_input, _)) = &self.input.payload() {
-            FileContentsImpl::new(payload_input.clone(), range)
+            if let Some(payload_ref) = &range.payload_ref {
+                let header: format::Header =
+                    read_entry_at(payload_input, payload_ref.offset).await?;
+                format::check_payload_header_and_size(&header, payload_ref.size)?;
+            }
+            Ok(FileContentsImpl::new(
+                payload_input.clone(),
+                range.content.clone(),
+            ))
         } else {
-            FileContentsImpl::new(self.input.archive().clone(), range)
+            Ok(FileContentsImpl::new(
+                self.input.archive().clone(),
+                range.content.clone(),
+            ))
         }
     }
 
@@ -758,7 +772,7 @@ impl<T: Clone + ReadAt> FileEntryImpl<T> {
     }
 
     /// For use with unsafe accessor methods.
-    pub fn content_range(&self) -> io::Result<Option<Range<u64>>> {
+    pub fn content_range(&self) -> io::Result<Option<ContentRange>> {
         match self.entry.kind {
             EntryKind::File { offset: None, .. } => {
                 io_bail!("cannot open file, reader provided no offset")
@@ -767,7 +781,10 @@ impl<T: Clone + ReadAt> FileEntryImpl<T> {
                 size,
                 offset: Some(offset),
                 payload_offset: None,
-            } => Ok(Some(offset..(offset + size))),
+            } => Ok(Some(ContentRange {
+                content: offset..(offset + size),
+                payload_ref: None,
+            })),
             // Payload offset beats regular offset if some
             EntryKind::File {
                 size,
@@ -775,7 +792,13 @@ impl<T: Clone + ReadAt> FileEntryImpl<T> {
                 payload_offset: Some(payload_offset),
             } => {
                 let start_offset = payload_offset + size_of::<format::Header>() as u64;
-                Ok(Some(start_offset..start_offset + size))
+                Ok(Some(ContentRange {
+                    content: start_offset..start_offset + size,
+                    payload_ref: Some(PayloadRef {
+                        offset: payload_offset,
+                        size,
+                    }),
+                }))
             }
             _ => Ok(None),
         }
@@ -786,9 +809,21 @@ impl<T: Clone + ReadAt> FileEntryImpl<T> {
             .content_range()?
             .ok_or_else(|| io_format_err!("not a file"))?;
         if let Some((ref payload_input, _)) = self.input.payload() {
-            Ok(FileContentsImpl::new(payload_input.clone(), range))
+            if let Some(payload_ref) = range.payload_ref {
+                let header: format::Header =
+                    read_entry_at(payload_input, payload_ref.offset).await?;
+                format::check_payload_header_and_size(&header, payload_ref.size)?;
+            }
+
+            Ok(FileContentsImpl::new(
+                payload_input.clone(),
+                range.content.clone(),
+            ))
         } else {
-            Ok(FileContentsImpl::new(self.input.archive().clone(), range))
+            Ok(FileContentsImpl::new(
+                self.input.archive().clone(),
+                range.content.clone(),
+            ))
         }
     }
 
diff --git a/src/accessor/sync.rs b/src/accessor/sync.rs
index df2ed23..63f0265 100644
--- a/src/accessor/sync.rs
+++ b/src/accessor/sync.rs
@@ -1,7 +1,6 @@
 //! Blocking `pxar` random access handling.
 
 use std::io;
-use std::ops::Range;
 use std::os::unix::fs::FileExt;
 use std::path::Path;
 use std::pin::Pin;
@@ -10,7 +9,7 @@ use std::task::Context;
 
 use crate::accessor::{self, cache::Cache, MaybeReady, ReadAt, ReadAtOperation};
 use crate::decoder::Decoder;
-use crate::format::GoodbyeItem;
+use crate::format::{ContentRange, GoodbyeItem};
 use crate::util::poll_result_once;
 use crate::{Entry, PxarVariant};
 
@@ -142,11 +141,14 @@ impl<T: Clone + ReadAt> Accessor<T> {
     ///
     /// This will provide a reader over an arbitrary range of the archive file, so unless this
     /// comes from a actual file entry data, the contents might not make much sense.
-    pub unsafe fn open_contents_at_range(&self, range: Range<u64>) -> FileContents<T> {
-        FileContents {
-            inner: unsafe { self.inner.open_contents_at_range(range) },
+    pub unsafe fn open_contents_at_range(
+        &self,
+        range: &ContentRange,
+    ) -> io::Result<FileContents<T>> {
+        Ok(FileContents {
+            inner: poll_result_once(unsafe { self.inner.open_contents_at_range(range) })?,
             at: 0,
-        }
+        })
     }
 
     /// Following a hardlink.
@@ -291,7 +293,7 @@ impl<T: Clone + ReadAt> FileEntry<T> {
     }
 
     /// For use with unsafe accessor methods.
-    pub fn content_range(&self) -> io::Result<Option<Range<u64>>> {
+    pub fn content_range(&self) -> io::Result<Option<ContentRange>> {
         self.inner.content_range()
     }
 
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel