[pbs-devel] [PATCH pxar 0/3] pxar: make payload reference offset checks stricter

[pbs-devel] [PATCH pxar 0/3] pxar: make payload reference offset checks stricter

[Christian Ebner]

This patch series aims to block the encoding of possible corrupt split
pxar archives due to the payload reference offsets not being strict
monotonically increasing. This is a requirement for the sequential
decoder to be able to restore the contents from a sequential stream.

This should also help to detect more corrupt encodings as reported in
the community forum and therefore narrow down the still lingering
logical issue, still under investigation.

First two patches are style cleanups only, the third contains the
stricter encoder check.

[0] https://forum.proxmox.com/threads/170211/

pxar:

Christian Ebner (3):
  tree wide: fix formatting via `cargo fmt`
  tree wide: elide explicit lifetimes reported by cargo clippy
  encoder: improve consistency check for payload reference offsets

 examples/pxarcmd.rs     |  2 +-
 src/accessor/aio.rs     |  4 +--
 src/accessor/mod.rs     |  6 ++--
 src/accessor/read_at.rs |  6 ++--
 src/accessor/sync.rs    |  8 ++---
 src/decoder/aio.rs      |  2 +-
 src/decoder/mod.rs      |  6 ++--
 src/decoder/sync.rs     |  2 +-
 src/encoder/aio.rs      |  4 +--
 src/encoder/mod.rs      | 76 +++++++++++++++++++++++++++++------------
 src/encoder/sync.rs     |  4 +--
 src/lib.rs              |  2 +-
 tests/compat.rs         |  2 +-
 tests/simple/fs.rs      |  8 ++---
 tests/simple/main.rs    |  4 +--
 15 files changed, 85 insertions(+), 51 deletions(-)


Summary over all repositories:
  15 files changed, 85 insertions(+), 51 deletions(-)

-- 
Generated by git-murpp 0.8.1


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

[pbs-devel] [PATCH pxar 1/3] tree wide: fix formatting via `cargo fmt`

[Christian Ebner]

Code format cleanup only, no functional changes.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 examples/pxarcmd.rs  | 2 +-
 src/accessor/aio.rs  | 2 +-
 src/accessor/mod.rs  | 2 +-
 src/accessor/sync.rs | 2 +-
 src/decoder/mod.rs   | 2 +-
 src/lib.rs           | 2 +-
 tests/compat.rs      | 2 +-
 tests/simple/fs.rs   | 8 ++++----
 tests/simple/main.rs | 4 ++--
 9 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/examples/pxarcmd.rs b/examples/pxarcmd.rs
index 0294eba..d0f7d97 100644
--- a/examples/pxarcmd.rs
+++ b/examples/pxarcmd.rs
@@ -5,9 +5,9 @@ use std::os::linux::fs::MetadataExt;
 use std::os::unix::ffi::OsStrExt;
 use std::path::{Path, PathBuf};
 
+use pxar::Metadata;
 use pxar::accessor::Accessor;
 use pxar::encoder::{Encoder, LinkOffset, SeqWrite};
-use pxar::Metadata;
 
 macro_rules! format_err {
     ($($msg:tt)+) => {
diff --git a/src/accessor/aio.rs b/src/accessor/aio.rs
index eb89f8f..7cd7f87 100644
--- a/src/accessor/aio.rs
+++ b/src/accessor/aio.rs
@@ -13,7 +13,7 @@ use std::pin::Pin;
 use std::sync::Arc;
 use std::task::{Context, Poll};
 
-use crate::accessor::{self, cache::Cache, ContentRange, MaybeReady, ReadAt, ReadAtOperation};
+use crate::accessor::{self, ContentRange, MaybeReady, ReadAt, ReadAtOperation, cache::Cache};
 use crate::decoder::aio::Decoder;
 use crate::format::GoodbyeItem;
 use crate::util;
diff --git a/src/accessor/mod.rs b/src/accessor/mod.rs
index 48605eb..dd8aece 100644
--- a/src/accessor/mod.rs
+++ b/src/accessor/mod.rs
@@ -5,7 +5,7 @@
 use std::ffi::{OsStr, OsString};
 use std::future::Future;
 use std::io;
-use std::mem::{self, size_of, size_of_val, MaybeUninit};
+use std::mem::{self, MaybeUninit, size_of, size_of_val};
 use std::ops::Range;
 use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::path::{Path, PathBuf};
diff --git a/src/accessor/sync.rs b/src/accessor/sync.rs
index 76e8c03..93b26d0 100644
--- a/src/accessor/sync.rs
+++ b/src/accessor/sync.rs
@@ -7,7 +7,7 @@ use std::pin::Pin;
 use std::sync::Arc;
 use std::task::Context;
 
-use crate::accessor::{self, cache::Cache, ContentRange, MaybeReady, ReadAt, ReadAtOperation};
+use crate::accessor::{self, ContentRange, MaybeReady, ReadAt, ReadAtOperation, cache::Cache};
 use crate::decoder::Decoder;
 use crate::format::GoodbyeItem;
 use crate::util::poll_result_once;
diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
index eca51b6..c6a0cda 100644
--- a/src/decoder/mod.rs
+++ b/src/decoder/mod.rs
@@ -7,7 +7,7 @@
 use std::ffi::OsString;
 use std::future::poll_fn;
 use std::io;
-use std::mem::{self, size_of, size_of_val, MaybeUninit};
+use std::mem::{self, MaybeUninit, size_of, size_of_val};
 use std::ops::Range;
 use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::path::{Path, PathBuf};
diff --git a/src/lib.rs b/src/lib.rs
index e0c5498..8c62127 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -21,7 +21,7 @@ pub mod decoder;
 pub mod encoder;
 
 #[doc(inline)]
-pub use format::{mode, Stat};
+pub use format::{Stat, mode};
 
 /// File metadata found in pxar archives.
 ///
diff --git a/tests/compat.rs b/tests/compat.rs
index c9e681a..d26e9bf 100644
--- a/tests/compat.rs
+++ b/tests/compat.rs
@@ -4,7 +4,7 @@ use std::io::{self, Read};
 
 use endian_trait::Endian;
 
-use pxar::{decoder, format, EntryKind};
+use pxar::{EntryKind, decoder, format};
 
 fn write_raw_struct<T: Endian, W: io::Write + ?Sized>(output: &mut W, data: T) -> io::Result<()> {
     let data = data.to_le();
diff --git a/tests/simple/fs.rs b/tests/simple/fs.rs
index 39d1294..1e3b913 100644
--- a/tests/simple/fs.rs
+++ b/tests/simple/fs.rs
@@ -2,14 +2,14 @@ use std::collections::HashMap;
 use std::io::Read;
 use std::path::{Path, PathBuf};
 
-use pxar::decoder::sync as decoder;
+use pxar::EntryKind as PxarEntryKind;
+use pxar::Metadata;
 use pxar::decoder::SeqRead;
+use pxar::decoder::sync as decoder;
 use pxar::encoder::sync as encoder;
 use pxar::encoder::{LinkOffset, SeqWrite};
 use pxar::format::acl::{self, Permissions};
-use pxar::format::{mode, Device};
-use pxar::EntryKind as PxarEntryKind;
-use pxar::Metadata;
+use pxar::format::{Device, mode};
 
 use crate::Error;
 
diff --git a/tests/simple/main.rs b/tests/simple/main.rs
index e403184..dcc5667 100644
--- a/tests/simple/main.rs
+++ b/tests/simple/main.rs
@@ -1,11 +1,11 @@
 use std::io::Read;
 use std::path::Path;
 
+use pxar::EntryKind as PxarEntryKind;
 use pxar::accessor::sync as accessor;
 use pxar::decoder::sync as decoder;
-use pxar::encoder::sync as encoder;
 use pxar::encoder::SeqWrite;
-use pxar::EntryKind as PxarEntryKind;
+use pxar::encoder::sync as encoder;
 
 macro_rules! format_err {
     ($($msg:tt)+) => {
-- 
2.47.2



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

[pbs-devel] [PATCH pxar 2/3] tree wide: elide explicit lifetimes reported by cargo clippy

[Christian Ebner]

Code style cleanup only, no functional changes.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/accessor/aio.rs     |  2 +-
 src/accessor/mod.rs     |  4 ++--
 src/accessor/read_at.rs |  6 +++---
 src/accessor/sync.rs    |  6 +++---
 src/decoder/aio.rs      |  2 +-
 src/decoder/mod.rs      |  4 ++--
 src/decoder/sync.rs     |  2 +-
 src/encoder/aio.rs      |  4 ++--
 src/encoder/mod.rs      | 10 +++++-----
 src/encoder/sync.rs     |  4 ++--
 10 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/src/accessor/aio.rs b/src/accessor/aio.rs
index 7cd7f87..827289a 100644
--- a/src/accessor/aio.rs
+++ b/src/accessor/aio.rs
@@ -317,7 +317,7 @@ pub struct DirEntry<'a, T: Clone + ReadAt> {
     inner: accessor::DirEntryImpl<'a, T>,
 }
 
-impl<'a, T: Clone + ReadAt> DirEntry<'a, T> {
+impl<T: Clone + ReadAt> DirEntry<'_, T> {
     /// Get the current file name.
     pub fn file_name(&self) -> &Path {
         self.inner.file_name()
diff --git a/src/accessor/mod.rs b/src/accessor/mod.rs
index dd8aece..3605fd9 100644
--- a/src/accessor/mod.rs
+++ b/src/accessor/mod.rs
@@ -112,7 +112,7 @@ where
 }
 
 /// Allow using trait objects for `T: ReadAt`
-impl<'d> ReadAt for &(dyn ReadAt + 'd) {
+impl ReadAt for &(dyn ReadAt + '_) {
     fn start_read_at<'a>(
         self: Pin<&'a Self>,
         cx: &mut Context,
@@ -873,7 +873,7 @@ pub(crate) struct DirEntryImpl<'a, T: Clone + ReadAt> {
     caches: Arc<Caches>,
 }
 
-impl<'a, T: Clone + ReadAt> DirEntryImpl<'a, T> {
+impl<T: Clone + ReadAt> DirEntryImpl<'_, T> {
     pub fn file_name(&self) -> &Path {
         &self.file_name
     }
diff --git a/src/accessor/read_at.rs b/src/accessor/read_at.rs
index d1d2e9b..4d2458b 100644
--- a/src/accessor/read_at.rs
+++ b/src/accessor/read_at.rs
@@ -73,7 +73,7 @@ pub struct ReadAtOperation<'a> {
     _marker: PhantomData<&'a mut [u8]>,
 }
 
-impl<'a> ReadAtOperation<'a> {
+impl ReadAtOperation<'_> {
     /// Create a new [`ReadAtOperation`].
     pub fn new<T: Into<Box<dyn Any + Send + Sync>>>(cookie: T) -> Self {
         Self {
@@ -112,7 +112,7 @@ impl<'a, T: ReadAt> ReadAtImpl<'a, T> {
     }
 }
 
-impl<'a, T: ReadAt> Future for ReadAtImpl<'a, T> {
+impl<T: ReadAt> Future for ReadAtImpl<'_, T> {
     type Output = io::Result<usize>;
 
     fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
@@ -132,7 +132,7 @@ impl<T: ReadAt> ReadAtState<'_, T> {
     }
 }
 
-impl<'a, T: ReadAt> Future for ReadAtState<'a, T> {
+impl<T: ReadAt> Future for ReadAtState<'_, T> {
     type Output = io::Result<usize>;
 
     fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
diff --git a/src/accessor/sync.rs b/src/accessor/sync.rs
index 93b26d0..ad82503 100644
--- a/src/accessor/sync.rs
+++ b/src/accessor/sync.rs
@@ -339,7 +339,7 @@ pub struct ReadDir<'a, T> {
     inner: accessor::ReadDirImpl<'a, T>,
 }
 
-impl<'a, T: Clone + ReadAt> ReadDir<'a, T> {
+impl<T: Clone + ReadAt> ReadDir<'_, T> {
     /// Efficient alternative to `Iterator::skip`.
     #[inline]
     pub fn skip(self, n: usize) -> Self {
@@ -367,7 +367,7 @@ impl<'a, T: Clone + ReadAt> Iterator for ReadDir<'a, T> {
     }
 }
 
-impl<'a, T: Clone + ReadAt> std::iter::FusedIterator for ReadDir<'a, T> {}
+impl<T: Clone + ReadAt> std::iter::FusedIterator for ReadDir<'_, T> {}
 
 /// A directory entry. When iterating through the contents of a directory we first get access to
 /// the file name. The remaining information can be decoded afterwards.
@@ -376,7 +376,7 @@ pub struct DirEntry<'a, T: Clone + ReadAt> {
     inner: accessor::DirEntryImpl<'a, T>,
 }
 
-impl<'a, T: Clone + ReadAt> DirEntry<'a, T> {
+impl<T: Clone + ReadAt> DirEntry<'_, T> {
     /// Get the current file name.
     pub fn file_name(&self) -> &Path {
         self.inner.file_name()
diff --git a/src/decoder/aio.rs b/src/decoder/aio.rs
index 19e7023..21ea571 100644
--- a/src/decoder/aio.rs
+++ b/src/decoder/aio.rs
@@ -109,7 +109,7 @@ mod tok {
         }
     }
 
-    impl<'a, T: crate::decoder::SeqRead> tokio::io::AsyncRead for Contents<'a, T> {
+    impl<T: crate::decoder::SeqRead> tokio::io::AsyncRead for Contents<'_, T> {
         fn poll_read(
             self: Pin<&mut Self>,
             cx: &mut Context<'_>,
diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
index c6a0cda..d62d030 100644
--- a/src/decoder/mod.rs
+++ b/src/decoder/mod.rs
@@ -68,7 +68,7 @@ pub trait SeqRead {
 }
 
 /// Allow using trait objects for generics taking a `SeqRead`:
-impl<'a> SeqRead for &mut (dyn SeqRead + 'a) {
+impl SeqRead for &mut (dyn SeqRead + '_) {
     fn poll_seq_read(
         self: Pin<&mut Self>,
         cx: &mut Context,
@@ -848,7 +848,7 @@ impl<'a, T: SeqRead> Contents<'a, T> {
     }
 }
 
-impl<'a, T: SeqRead> SeqRead for Contents<'a, T> {
+impl<T: SeqRead> SeqRead for Contents<'_, T> {
     fn poll_seq_read(
         mut self: Pin<&mut Self>,
         cx: &mut Context,
diff --git a/src/decoder/sync.rs b/src/decoder/sync.rs
index 1116fe8..2c6ab4e 100644
--- a/src/decoder/sync.rs
+++ b/src/decoder/sync.rs
@@ -133,7 +133,7 @@ pub struct Contents<'a, T: SeqRead> {
     inner: decoder::Contents<'a, T>,
 }
 
-impl<'a, T: SeqRead> io::Read for Contents<'a, T> {
+impl<T: SeqRead> io::Read for Contents<'_, T> {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
         poll_result_once(super::seq_read(&mut self.inner, buf))
     }
diff --git a/src/encoder/aio.rs b/src/encoder/aio.rs
index 8973402..66cb7ed 100644
--- a/src/encoder/aio.rs
+++ b/src/encoder/aio.rs
@@ -220,7 +220,7 @@ pub struct File<'a, S: SeqWrite> {
     inner: encoder::FileImpl<'a, S>,
 }
 
-impl<'a, S: SeqWrite> File<'a, S> {
+impl<S: SeqWrite> File<'_, S> {
     /// Get the file offset to be able to reference it with `add_hardlink`.
     pub fn file_offset(&self) -> LinkOffset {
         self.inner.file_offset()
@@ -238,7 +238,7 @@ impl<'a, S: SeqWrite> File<'a, S> {
 }
 
 #[cfg(feature = "tokio-io")]
-impl<'a, S: SeqWrite> tokio::io::AsyncWrite for File<'a, S> {
+impl<S: SeqWrite> tokio::io::AsyncWrite for File<'_, S> {
     fn poll_write(self: Pin<&mut Self>, cx: &mut Context, data: &[u8]) -> Poll<io::Result<usize>> {
         unsafe { self.map_unchecked_mut(|this| &mut this.inner) }.poll_write(cx, data)
     }
diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs
index 0c17aa3..4e7daef 100644
--- a/src/encoder/mod.rs
+++ b/src/encoder/mod.rs
@@ -292,7 +292,7 @@ pub(crate) enum EncoderOutput<'a, T> {
     Borrowed(&'a mut T),
 }
 
-impl<'a, T> std::convert::AsMut<T> for EncoderOutput<'a, T> {
+impl<T> std::convert::AsMut<T> for EncoderOutput<'_, T> {
     fn as_mut(&mut self) -> &mut T {
         match self {
             EncoderOutput::Owned(o) => o,
@@ -301,7 +301,7 @@ impl<'a, T> std::convert::AsMut<T> for EncoderOutput<'a, T> {
     }
 }
 
-impl<'a, T> std::convert::From<T> for EncoderOutput<'a, T> {
+impl<T> std::convert::From<T> for EncoderOutput<'_, T> {
     fn from(t: T) -> Self {
         EncoderOutput::Owned(t)
     }
@@ -1090,7 +1090,7 @@ pub(crate) struct FileImpl<'a, S: SeqWrite> {
     parent: &'a mut EncoderState,
 }
 
-impl<'a, S: SeqWrite> Drop for FileImpl<'a, S> {
+impl<S: SeqWrite> Drop for FileImpl<'_, S> {
     fn drop(&mut self) {
         if self.remaining_size != 0 {
             self.parent.add_error(EncodeError::IncompleteFile);
@@ -1100,7 +1100,7 @@ impl<'a, S: SeqWrite> Drop for FileImpl<'a, S> {
     }
 }
 
-impl<'a, S: SeqWrite> FileImpl<'a, S> {
+impl<S: SeqWrite> FileImpl<'_, S> {
     /// Get the file offset to be able to reference it with `add_hardlink`.
     pub fn file_offset(&self) -> LinkOffset {
         LinkOffset(self.goodbye_item.offset)
@@ -1200,7 +1200,7 @@ impl<'a, S: SeqWrite> FileImpl<'a, S> {
 }
 
 #[cfg(feature = "tokio-io")]
-impl<'a, S: SeqWrite> tokio::io::AsyncWrite for FileImpl<'a, S> {
+impl<S: SeqWrite> tokio::io::AsyncWrite for FileImpl<'_, S> {
     fn poll_write(self: Pin<&mut Self>, cx: &mut Context, buf: &[u8]) -> Poll<io::Result<usize>> {
         FileImpl::poll_write(self, cx, buf)
     }
diff --git a/src/encoder/sync.rs b/src/encoder/sync.rs
index af2f902..ee04b07 100644
--- a/src/encoder/sync.rs
+++ b/src/encoder/sync.rs
@@ -221,14 +221,14 @@ pub struct File<'a, S: SeqWrite> {
     inner: encoder::FileImpl<'a, S>,
 }
 
-impl<'a, S: SeqWrite> File<'a, S> {
+impl<S: SeqWrite> File<'_, S> {
     /// Get the file offset to be able to reference it with `add_hardlink`.
     pub fn file_offset(&self) -> LinkOffset {
         self.inner.file_offset()
     }
 }
 
-impl<'a, S: SeqWrite> io::Write for File<'a, S> {
+impl<S: SeqWrite> io::Write for File<'_, S> {
     fn write(&mut self, data: &[u8]) -> io::Result<usize> {
         poll_result_once(self.inner.write(data))
     }
-- 
2.47.2



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

[pbs-devel] [PATCH pxar 3/3] encoder: improve consistency check for payload reference offsets

[Christian Ebner]

The pxar encoder for split metadata and payload archives cannot keep
exactly track of the payload stream and payload reference offset to
be in sync since it must allow for injecting payloads steming from
reused chunks in PBS. Therefore, currently it is only checked that the
offset to encode in the payload reference is greater than the current
payload writer position (the chunk data is always injected after
advancing the metadata archive).

This check is however not good enough to protect against encoding an
archive with not strict monotonically increasing payload reference
offset, which cannot be restored by the sequential decoder.

Guard against this by keeping track of the previously encoded payload
offset and check for the new one to be always greater. Move the
pre-existing check together with the stricter check into a common
helper and us it also for checking the offsets when creating files
being encoded with split pxar archives, not just when encoding a
reused file in the metadata archive as pxar payload reference.

This will avoid the encoding of corrupt archives as reported in the
community forum [0].

[0] https://forum.proxmox.com/threads/170211/

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/encoder/mod.rs | 66 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 50 insertions(+), 16 deletions(-)

diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs
index 4e7daef..131cd69 100644
--- a/src/encoder/mod.rs
+++ b/src/encoder/mod.rs
@@ -243,6 +243,9 @@ struct EncoderState {
     /// Track the bytes written to the payload writer
     payload_write_position: u64,
 
+    /// Previously generated payload offset
+    previous_payload_offset: Option<PayloadOffset>,
+
     /// Mark the encoder state as correctly finished, ready to be dropped
     finished: bool,
 }
@@ -258,6 +261,46 @@ impl EncoderState {
         self.payload_write_position
     }
 
+    #[inline]
+    fn previous_payload_offset(&self) -> Option<PayloadOffset> {
+        self.previous_payload_offset
+    }
+
+    /// Generate a PayloadRef from the given payload offset and file size.
+    ///
+    /// Checks if the provided payload offset is greater than the current payload writeer position
+    /// and larger than the previously checked payload offset. Both are required for the sequential
+    /// decoder to be able to restore contents.
+    fn payload_ref_from(
+        &mut self,
+        payload_offset: PayloadOffset,
+        size: u64,
+    ) -> io::Result<PayloadRef> {
+        let payload_position = self.payload_position();
+        if payload_offset.raw() < payload_position {
+            io_bail!(
+                "offset smaller than current position: {} < {payload_position}",
+                payload_offset.raw()
+            );
+        }
+
+        if let Some(previous_payload_offset) = self.previous_payload_offset() {
+            if payload_offset <= previous_payload_offset {
+                io_bail!(
+                    "unexpected payload offset {} not larger than previous offset {}",
+                    payload_offset.raw(),
+                    previous_payload_offset.raw(),
+                );
+            }
+        }
+        self.previous_payload_offset = Some(payload_offset);
+
+        Ok(PayloadRef {
+            offset: payload_offset.raw(),
+            size,
+        })
+    }
+
     fn merge_error(&mut self, error: Option<EncodeError>) {
         // one error is enough:
         if self.encode_error.is_none() {
@@ -454,17 +497,13 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
         if let Some(payload_output) = output.payload_mut() {
             // payload references must point to the position prior to the payload header,
             // separating payload entries in the payload stream
-            let payload_position = state.payload_position();
+            let payload_position = PayloadOffset(state.payload_position());
+            let payload_ref = state.payload_ref_from(payload_position, file_size)?;
 
             let header = format::Header::with_content_size(format::PXAR_PAYLOAD, file_size);
             header.check_header_size()?;
             seq_write_struct(payload_output, header, &mut state.payload_write_position).await?;
 
-            let payload_ref = PayloadRef {
-                offset: payload_position,
-                size: file_size,
-            };
-
             seq_write_pxar_entry(
                 output.archive_mut(),
                 format::PXAR_PAYLOAD_REF,
@@ -566,16 +605,9 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
             io_bail!("unable to add payload reference");
         }
 
-        let offset = payload_offset.raw();
-        let payload_position = self.state()?.payload_position();
-        if offset < payload_position {
-            io_bail!("offset smaller than current position: {offset} < {payload_position}");
-        }
-
-        let payload_ref = PayloadRef {
-            offset,
-            size: file_size,
-        };
+        let payload_ref = self
+            .state_mut()?
+            .payload_ref_from(payload_offset, file_size)?;
         let this_offset: LinkOffset = self
             .add_file_entry(
                 Some(metadata),
@@ -749,6 +781,7 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
         // the child will write to OUR state now:
         let write_position = state.position();
         let payload_write_position = state.payload_position();
+        let previous_payload_offset = state.previous_payload_offset();
 
         self.state.push(EncoderState {
             items: Vec::new(),
@@ -759,6 +792,7 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
             file_hash,
             write_position,
             payload_write_position,
+            previous_payload_offset,
             finished: false,
         });
 
-- 
2.47.2



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

[pbs-devel] applied-series: [PATCH pxar 0/3] pxar: make payload reference offset checks stricter

[Wolfgang Bumiller]

applied, thanks

On Wed, Aug 27, 2025 at 11:46:30AM +0200, Christian Ebner wrote:
> This patch series aims to block the encoding of possible corrupt split
> pxar archives due to the payload reference offsets not being strict
> monotonically increasing. This is a requirement for the sequential
> decoder to be able to restore the contents from a sequential stream.
> 
> This should also help to detect more corrupt encodings as reported in
> the community forum and therefore narrow down the still lingering
> logical issue, still under investigation.
> 
> First two patches are style cleanups only, the third contains the
> stricter encoder check.
> 
> [0] https://forum.proxmox.com/threads/170211/
> 
> pxar:
> 
> Christian Ebner (3):
>   tree wide: fix formatting via `cargo fmt`
>   tree wide: elide explicit lifetimes reported by cargo clippy
>   encoder: improve consistency check for payload reference offsets
> 
>  examples/pxarcmd.rs     |  2 +-
>  src/accessor/aio.rs     |  4 +--
>  src/accessor/mod.rs     |  6 ++--
>  src/accessor/read_at.rs |  6 ++--
>  src/accessor/sync.rs    |  8 ++---
>  src/decoder/aio.rs      |  2 +-
>  src/decoder/mod.rs      |  6 ++--
>  src/decoder/sync.rs     |  2 +-
>  src/encoder/aio.rs      |  4 +--
>  src/encoder/mod.rs      | 76 +++++++++++++++++++++++++++++------------
>  src/encoder/sync.rs     |  4 +--
>  src/lib.rs              |  2 +-
>  tests/compat.rs         |  2 +-
>  tests/simple/fs.rs      |  8 ++---
>  tests/simple/main.rs    |  4 +--
>  15 files changed, 85 insertions(+), 51 deletions(-)
> 
> 
> Summary over all repositories:
>   15 files changed, 85 insertions(+), 51 deletions(-)
> 
> -- 
> Generated by git-murpp 0.8.1


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