From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id D7D981FF0AA for ; Fri, 21 Aug 2026 16:03:11 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 7AEBB215ED; Fri, 21 Aug 2026 16:03:08 +0200 (CEST) From: "Max R. Carrara" To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v2 08/10] tape: blocked_{reader,writer}: remove haphazard `unsafe` blocks Date: Fri, 21 Aug 2026 16:02:32 +0200 Message-ID: <20260821140238.615302-9-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260821140238.615302-1-m.carrara@proxmox.com> References: <20260821140238.615302-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787320948429 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.662 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: GC2OSFMK3EHFOK7QQGQG73PEZPORM6RS X-Message-ID-Hash: GC2OSFMK3EHFOK7QQGQG73PEZPORM6RS X-MailFrom: m.carrara@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Both the `BlockedReader` and `BlockedWriter` structs use private helpers that cast their `TapeBlock` to a (mutable) byte slice using an `unsafe` block for reading and writing a tape block, respectively. Neither `unsafe` block is prefixed with a "// SAFETY: ..." comment, nor should these casts be done inline in the first place. Instead, implement these casts as methods on `TapeBlock` directly, with either `unsafe` block being preceded with a SAFETY comment. Signed-off-by: Max R. Carrara --- pbs-tape/src/blocked_reader.rs | 9 +-------- pbs-tape/src/blocked_writer.rs | 8 +------- pbs-tape/src/tape_block.rs | 31 +++++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/pbs-tape/src/blocked_reader.rs b/pbs-tape/src/blocked_reader.rs index 6b1e3c692..6f5f87aa7 100644 --- a/pbs-tape/src/blocked_reader.rs +++ b/pbs-tape/src/blocked_reader.rs @@ -95,14 +95,7 @@ impl BlockedReader { } fn read_block_frame(tape_block: &mut TapeBlock, reader: &mut R) -> Result<(), BlockReadError> { - let data = unsafe { - std::slice::from_raw_parts_mut( - (tape_block as *mut TapeBlock) as *mut u8, - TapeBlock::SIZE, - ) - }; - - let bytes = reader.read_block(data)?; + let bytes = reader.read_block(tape_block.as_bytes_mut())?; if bytes != TapeBlock::SIZE { return Err(proxmox_lang::io_format_err!("got wrong block size").into()); diff --git a/pbs-tape/src/blocked_writer.rs b/pbs-tape/src/blocked_writer.rs index 0d5d10147..44ff15ae0 100644 --- a/pbs-tape/src/blocked_writer.rs +++ b/pbs-tape/src/blocked_writer.rs @@ -46,13 +46,7 @@ impl BlockedWriter { } fn write_block(tape_block: &TapeBlock, writer: &mut W) -> Result { - let data = unsafe { - std::slice::from_raw_parts( - (tape_block as *const TapeBlock) as *const u8, - TapeBlock::SIZE, - ) - }; - writer.write_block(data) + writer.write_block(tape_block.as_bytes()) } fn write_eof(&mut self) -> Result<(), std::io::Error> { diff --git a/pbs-tape/src/tape_block.rs b/pbs-tape/src/tape_block.rs index c73efafa7..c6ff98395 100644 --- a/pbs-tape/src/tape_block.rs +++ b/pbs-tape/src/tape_block.rs @@ -117,4 +117,35 @@ impl TapeBlock { pub fn payload_mut(&mut self) -> &mut [u8] { &mut self.payload } + + /// Returns the entirety of the tape block, meaning both its header and data + /// payload, as a byte slice. + pub fn as_bytes(&self) -> &[u8] { + // SAFETY: + // - Since `self` is a reference, we can convert it to a pointer without + // any concerns. The resulting pointer is always valid and non-null. + // - The pointer used here is not used or aliased anywhere else. + // - We allocated `*self` with a total size of `Self::SIZE` earlier, + // meaning that the resulting slice never goes out of bounds. + // - The resulting slice never outlives `self`. + unsafe { std::slice::from_raw_parts((self as *const _) as *const u8, Self::SIZE) } + } + + /// Returns the entirety of the tape block, meaning both its header and data + /// payload, as a mutable byte slice. + /// + /// While this method in itself is safe, be aware that it nevertheless + /// allows you to overwrite the tape block's header fields. + pub fn as_bytes_mut(&mut self) -> &mut [u8] { + // SAFETY: + // - Since `self` is a reference, we can convert it to a pointer without + // any concerns. The resulting pointer is always valid and non-null. + // - The pointer used here is not used or aliased anywhere else. + // - Since we uniquely borrow `self`, we may cast `self` to a `* mut` + // and use it to acquire a mutable slice. + // - We allocated `*self` with a total size of `Self::SIZE` earlier, + // meaning that the resulting slice never goes out of bounds. + // - The resulting slice never outlives `self`. + unsafe { std::slice::from_raw_parts_mut((self as *mut _) as *mut u8, Self::SIZE) } + } } -- 2.47.3