From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A58F477E84 for ; Wed, 21 Jul 2021 16:05:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 94F8F1608D for ; Wed, 21 Jul 2021 16:04:54 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 9AC6F16079 for ; Wed, 21 Jul 2021 16:04:52 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 539A740609 for ; Wed, 21 Jul 2021 16:04:52 +0200 (CEST) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Wed, 21 Jul 2021 16:04:51 +0200 Message-Id: <20210721140451.1839470-4-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210721140451.1839470-1-d.csapak@proxmox.com> References: <20210721140451.1839470-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.547 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup 4/4] tape: changer: sg_pt: correctly consume data in decode_element_status_page X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Jul 2021 14:05:24 -0000 instead of 'blindly' trusting the changer to deliver the fields written in the specification, trust the length data it returns in the header. now we count the data we consume from it, and do not error out if some fields at the end are missing (we do not need most of them anyway) this also makes the code to read the rest of the page a bit easier, since we already counted how much should be left Signed-off-by: Dominik Csapak --- src/tape/changer/sg_pt_changer.rs | 99 +++++++++++++++++++------------ 1 file changed, 62 insertions(+), 37 deletions(-) diff --git a/src/tape/changer/sg_pt_changer.rs b/src/tape/changer/sg_pt_changer.rs index a15d8192..b0417f82 100644 --- a/src/tape/changer/sg_pt_changer.rs +++ b/src/tape/changer/sg_pt_changer.rs @@ -657,23 +657,32 @@ fn decode_element_status_page( if reader.is_empty() { break; } - if reader.len() < (subhead.descriptor_length as usize) { + let mut remaining_size = subhead.descriptor_length as usize; + if reader.len() < remaining_size { break; } - let len_before = reader.len(); - match subhead.element_type_code { 1 => { let desc: TransportDescriptor = unsafe { reader.read_be_value()? }; + remaining_size -= std::mem::size_of::(); let full = (desc.flags1 & 1) != 0; let volume_tag = subhead.parse_optional_volume_tag(&mut reader, full)?; + if volume_tag.is_some() { + remaining_size -= SCSI_VOLUME_TAG_LEN; + } - subhead.skip_alternate_volume_tag(&mut reader)?; + if remaining_size >= SCSI_VOLUME_TAG_LEN { + subhead.skip_alternate_volume_tag(&mut reader)?; + remaining_size -= SCSI_VOLUME_TAG_LEN; + } - let mut reserved = [0u8; 4]; - reader.read_exact(&mut reserved)?; + if remaining_size >= 4 { + let mut reserved = [0u8; 4]; + reader.read_exact(&mut reserved)?; + remaining_size -= 4; + } result.last_element_address = Some(desc.element_address); @@ -685,14 +694,24 @@ fn decode_element_status_page( } 2 | 3 => { let desc: StorageDescriptor = unsafe { reader.read_be_value()? }; + remaining_size -= std::mem::size_of::(); let full = (desc.flags1 & 1) != 0; let volume_tag = subhead.parse_optional_volume_tag(&mut reader, full)?; + if volume_tag.is_some() { + remaining_size -= SCSI_VOLUME_TAG_LEN; + } - subhead.skip_alternate_volume_tag(&mut reader)?; + if remaining_size >= SCSI_VOLUME_TAG_LEN { + subhead.skip_alternate_volume_tag(&mut reader)?; + remaining_size -= SCSI_VOLUME_TAG_LEN; + } - let mut reserved = [0u8; 4]; - reader.read_exact(&mut reserved)?; + if remaining_size >= 4 { + let mut reserved = [0u8; 4]; + reader.read_exact(&mut reserved)?; + remaining_size -= 4; + } result.last_element_address = Some(desc.element_address); @@ -714,6 +733,7 @@ fn decode_element_status_page( } 4 => { let desc: TransferDescriptor = unsafe { reader.read_be_value()? }; + remaining_size -= std::mem::size_of::(); let loaded_slot = if (desc.flags2 & 128) != 0 { // SValid Some(desc.source_storage_element_address as u64) @@ -723,30 +743,39 @@ fn decode_element_status_page( let full = (desc.flags1 & 1) != 0; let volume_tag = subhead.parse_optional_volume_tag(&mut reader, full)?; + if volume_tag.is_some() { + remaining_size -= SCSI_VOLUME_TAG_LEN; + } - subhead.skip_alternate_volume_tag(&mut reader)?; - - let dvcid: DvcidHead = unsafe { reader.read_be_value()? }; + if remaining_size >= SCSI_VOLUME_TAG_LEN { + subhead.skip_alternate_volume_tag(&mut reader)?; + remaining_size -= SCSI_VOLUME_TAG_LEN; + } - let (drive_serial_number, vendor, model) = match (dvcid.code_set, dvcid.identifier_type) { - (2, 0) => { // Serial number only (Quantum Superloader3 uses this) - let serial = reader.read_exact_allocated(dvcid.identifier_len as usize)?; - let serial = scsi_ascii_to_string(&serial); - (Some(serial), None, None) - } - (2, 1) => { - if dvcid.identifier_len != 34 { - bail!("got wrong DVCID length"); + let (drive_serial_number, vendor, model) = if remaining_size >= std::mem::size_of::() { + let dvcid: DvcidHead = unsafe { reader.read_be_value()? }; + match (dvcid.code_set, dvcid.identifier_type) { + (2, 0) => { // Serial number only (Quantum Superloader3 uses this) + let serial = reader.read_exact_allocated(dvcid.identifier_len as usize)?; + let serial = scsi_ascii_to_string(&serial); + (Some(serial), None, None) + } + (2, 1) => { + if dvcid.identifier_len != 34 { + bail!("got wrong DVCID length"); + } + let vendor = reader.read_exact_allocated(8)?; + let vendor = scsi_ascii_to_string(&vendor); + let model = reader.read_exact_allocated(16)?; + let model = scsi_ascii_to_string(&model); + let serial = reader.read_exact_allocated(10)?; + let serial = scsi_ascii_to_string(&serial); + (Some(serial), Some(vendor), Some(model)) } - let vendor = reader.read_exact_allocated(8)?; - let vendor = scsi_ascii_to_string(&vendor); - let model = reader.read_exact_allocated(16)?; - let model = scsi_ascii_to_string(&model); - let serial = reader.read_exact_allocated(10)?; - let serial = scsi_ascii_to_string(&serial); - (Some(serial), Some(vendor), Some(model)) + _ => (None, None, None), } - _ => (None, None, None), + } else { + (None, None, None) }; result.last_element_address = Some(desc.element_address); @@ -766,15 +795,11 @@ fn decode_element_status_page( // we have to consume the whole descriptor size, else // our position in the reader is not correct - let len_after = reader.len(); - let have_read = len_before - len_after; - let desc_len = subhead.descriptor_length as usize; - if desc_len > have_read { - let mut left_to_read = desc_len - have_read; - if left_to_read > len_after { - left_to_read = len_after; // reader has not enough data? + if remaining_size > 0 { + if remaining_size > reader.len() { + remaining_size = reader.len(); // reader has not enough data? } - let _ = reader.read_exact_allocated(left_to_read)?; + let _ = reader.read_exact_allocated(remaining_size)?; } } } -- 2.30.2