public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Max R. Carrara" <m.carrara@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH proxmox-backup 0/2] Fix Undefined Behavior in Tape Block Header Deallocation
Date: Wed,  5 Aug 2026 16:13:26 +0200	[thread overview]
Message-ID: <20260805141422.4187158-1-m.carrara@proxmox.com> (raw)

Fix Undefined Behavior in Tape Block Header Deallocation - v1
=============================================================

We allocate `BlockHeader` in `pbs-tape` with an alignment equal
to the page size, but because it uses packed / 1-byte alignment
(`#[repr(C, packed)]` to be precise), the compiler will treat it as if
it was allocated with 1-byte alignment.

Miri [0] will flag this as undefined behavior:

    error: Undefined Behavior: incorrect layout on deallocation:
    alloc46398 has size 262144 and alignment 4096, but gave size 262144
    and alignment 1

This is because Rust cares about a memory region's alignment on
deallocation, even if the underlying allocator does not -- hence why
this hasn't actually been a problem for us. Still, there is no guarantee
that this will not change at some point in the future.

This is fixed in patch #1. Patch #2 just renames variables to fit the
new implementation better, which I didn't want to include in patch #1,
as it didn't really fit (and it would make the diff ugly). If you feel
like patch #2 is superfluous, feel free to drop it.

Special thanks to @Robert, who brought this to my attention and totally
managed to nerd-snipe me with this. I have added a respective git
trailer in patch #1.

Testing
-------

Would be great if somebody with a working tape storage could give this a
spin -- miri does not report any UB anymore and the tests we have pass,
but some smoke-testing would nevertheless be appreciated. My virtual
tape library has unfortunately borked itself, and I haven't come around
to un-borking it yet.

[0]: https://github.com/rust-lang/miri

Summary of Changes
------------------


proxmox-backup:

Max R. Carrara (2):
  pbs-tape: fix undefined behavior in block header deallocation
  pbs-tape: rename `buffer` to `header`

 pbs-tape/src/blocked_reader.rs |  58 +++---
 pbs-tape/src/blocked_writer.rs |  47 ++---
 pbs-tape/src/lib.rs            | 352 +++++++++++++++++++++++++++------
 3 files changed, 346 insertions(+), 111 deletions(-)


Summary over all repositories:
  3 files changed, 346 insertions(+), 111 deletions(-)

--
Generated by murpp 0.12.0




             reply	other threads:[~2026-08-05 14:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 14:13 Max R. Carrara [this message]
2026-08-05 14:13 ` [PATCH proxmox-backup 1/2] pbs-tape: fix undefined behavior in block header deallocation Max R. Carrara
2026-08-05 14:13 ` [PATCH proxmox-backup 2/2] pbs-tape: rename `buffer` to `header` Max R. Carrara
2026-08-05 14:15 ` [PATCH proxmox-backup 0/2] Fix Undefined Behavior in Tape Block Header Deallocation Max R. Carrara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260805141422.4187158-1-m.carrara@proxmox.com \
    --to=m.carrara@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal