[pbs-devel] [PATCH proxmox-backup v2 5/6] tape: changer: sg_pt: correctly consume data in decode_element_status_page
Dominik Csapak
d.csapak at proxmox.com
Wed Jul 21 17:10:42 CEST 2021
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)
also omit the reading of not necessary fields at thend of the descriptor,
since we read the whole remaining descriptor to get the correct
position 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 <d.csapak at proxmox.com>
---
changes from v1:
* correctly subtract the volume tag size when it was read, not only
when the volume tag had content (empty slots!)
* correctly reduce 'remaining_size' when reading the dvcid fields
* drop the reading of the 'reserved' field at the end, we read
the remaining descriptor anyway
src/tape/changer/sg_pt_changer.rs | 93 +++++++++++++++++--------------
1 file changed, 52 insertions(+), 41 deletions(-)
diff --git a/src/tape/changer/sg_pt_changer.rs b/src/tape/changer/sg_pt_changer.rs
index 9938dfe1..c8510aff 100644
--- a/src/tape/changer/sg_pt_changer.rs
+++ b/src/tape/changer/sg_pt_changer.rs
@@ -661,23 +661,24 @@ 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::<TransportDescriptor>();
let full = (desc.flags1 & 1) != 0;
- let (volume_tag, _) = subhead.parse_optional_volume_tag(&mut reader, full)?;
-
- subhead.parse_alternate_volume_tag(&mut reader)?;
+ let (volume_tag, read) = subhead.parse_optional_volume_tag(&mut reader, full)?;
+ remaining_size -= read;
- let mut reserved = [0u8; 4];
- reader.read_exact(&mut reserved)?;
+ if remaining_size >= SCSI_VOLUME_TAG_LEN {
+ let (_, read) = subhead.parse_alternate_volume_tag(&mut reader)?;
+ remaining_size -= read;
+ }
result.last_element_address = Some(desc.element_address);
@@ -689,14 +690,16 @@ fn decode_element_status_page(
}
2 | 3 => {
let desc: StorageDescriptor = unsafe { reader.read_be_value()? };
+ remaining_size -= std::mem::size_of::<StorageDescriptor>();
let full = (desc.flags1 & 1) != 0;
- let (volume_tag, _) = subhead.parse_optional_volume_tag(&mut reader, full)?;
+ let (volume_tag, read) = subhead.parse_optional_volume_tag(&mut reader, full)?;
+ remaining_size -= read;
- subhead.parse_alternate_volume_tag(&mut reader)?;
-
- let mut reserved = [0u8; 4];
- reader.read_exact(&mut reserved)?;
+ if remaining_size >= SCSI_VOLUME_TAG_LEN {
+ let (_, read) = subhead.parse_alternate_volume_tag(&mut reader)?;
+ remaining_size -= read;
+ }
result.last_element_address = Some(desc.element_address);
@@ -718,6 +721,7 @@ fn decode_element_status_page(
}
4 => {
let desc: TransferDescriptor = unsafe { reader.read_be_value()? };
+ remaining_size -= std::mem::size_of::<TransferDescriptor>();
let loaded_slot = if (desc.flags2 & 128) != 0 { // SValid
Some(desc.source_storage_element_address as u64)
@@ -726,31 +730,42 @@ fn decode_element_status_page(
};
let full = (desc.flags1 & 1) != 0;
- let (volume_tag, _) = subhead.parse_optional_volume_tag(&mut reader, full)?;
+ let (volume_tag, read) = subhead.parse_optional_volume_tag(&mut reader, full)?;
+ remaining_size -= read;
- subhead.parse_alternate_volume_tag(&mut reader)?;
+ if remaining_size >= SCSI_VOLUME_TAG_LEN {
+ let (_, read) = subhead.parse_alternate_volume_tag(&mut reader)?;
+ remaining_size -= read;
+ }
- let dvcid: DvcidHead = unsafe { reader.read_be_value()? };
+ let (drive_serial_number, vendor, model) = if remaining_size >= std::mem::size_of::<DvcidHead>() {
+ let dvcid: DvcidHead = unsafe { reader.read_be_value()? };
+ remaining_size -= std::mem::size_of::<DvcidHead>();
- 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");
+ 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)?;
+ remaining_size -= dvcid.identifier_len as usize;
+ let serial = scsi_ascii_to_string(&serial);
+ (Some(serial), None, None)
}
- 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))
+ (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);
+ remaining_size -= 8 + 16 + 10;
+ (Some(serial), Some(vendor), Some(model))
+ }
+ _ => (None, None, None),
}
- _ => (None, None, None),
+ } else {
+ (None, None, None)
};
result.last_element_address = Some(desc.element_address);
@@ -770,15 +785,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
More information about the pbs-devel
mailing list