pldm-fw: ua: send actual component count in RequestUpdate

request_update() hardcoded the RequestUpdate NumberOfComponents field
to 1. DSP0267 Table 27 defines this field as "the number of components
that will be passed to the FD during the update", and the FD may use it
to compare against the number of PassComponentTable/UpdateComponent
commands it receives. When a package applies more than one component,
the UA passes all of them (pass_component_table and
update_components_progress iterate over update.components) while still
announcing only 1, which a conformant FD can reject as a mismatch.

Derive the count from update.components, the same list driven through
the rest of the update flow, so the announced value matches what is
actually sent. The value is fallibly converted to u16 to guard the
(practically impossible) >65535 component case rather than truncating.

Add a unit test (request_update_reports_actual_component_count) that
drives request_update over a mock MCTP ReqChannel and asserts the
RequestUpdate NumberOfComponents field reflects the actual component
count, along with the cfg(test) package-builder and temp-file helpers it
depends on.

Signed-off-by: Brian Carr <brcarr@nvidia.com>

Author: brcarr-nv

pldm-fw/src/pkg.rs

@@ -313,3 +313,137 @@ impl Package {
         Ok(self.file.read_at(buf, file_offset)?)
     }
 }
+
+/// Write `bytes` to a fresh temporary file and return a read handle to it.
+///
+/// The file is unlinked immediately; the returned handle keeps it alive, so
+/// the on-disk entry is cleaned up automatically once the handle is dropped.
+#[cfg(test)]
+pub(crate) fn temp_file_with(bytes: &[u8]) -> std::fs::File {
+    use std::io::Write;
+    use std::sync::atomic::{AtomicU64, Ordering};
+
+    static CTR: AtomicU64 = AtomicU64::new(0);
+    let n = CTR.fetch_add(1, Ordering::Relaxed);
+    let mut path = std::env::temp_dir();
+    path.push(format!("pldm-fw-test-{}-{}.bin", std::process::id(), n));
+
+    {
+        let mut f = std::fs::File::create(&path).unwrap();
+        f.write_all(bytes).unwrap();
+        f.flush().unwrap();
+    }
+    let f = std::fs::File::open(&path).unwrap();
+    // The open handle keeps the inode alive after unlink.
+    let _ = std::fs::remove_file(&path);
+    f
+}
+
+/// Build the bytes of a minimal, well-formed v1.1.x firmware update package.
+///
+/// The package describes a single device identified by one PCI Vendor ID
+/// descriptor (`vid`), whose component bitmap selects every supplied
+/// component. Each entry of `components` becomes a component image appended
+/// after the (CRC-protected) package header, with matching file offset/size
+/// fields.
+#[cfg(test)]
+pub(crate) fn build_v11_package(vid: u16, components: &[&[u8]]) -> Vec<u8> {
+    let ncomp = components.len();
+    assert!(ncomp >= 1);
+
+    // --- single device record ---
+    let bitmap_bytes = ncomp.div_ceil(8);
+    let mut bitmap = vec![0u8; bitmap_bytes];
+    for i in 0..ncomp {
+        bitmap[i / 8] |= 1u8 << (i % 8);
+    }
+    let set_ver = b"0000";
+
+    let mut descs = Vec::new();
+    descs.extend_from_slice(&0x0000u16.to_le_bytes()); // type: PCI Vendor ID
+    descs.extend_from_slice(&2u16.to_le_bytes()); // length
+    descs.extend_from_slice(&vid.to_le_bytes()); // data
+    let desc_count = 1u8;
+    let pkg_data_len = 0u16;
+
+    let mut rec_body = Vec::new();
+    rec_body.extend_from_slice(&bitmap);
+    rec_body.extend_from_slice(set_ver);
+    rec_body.extend_from_slice(&descs);
+    let rec_len = (11 + rec_body.len()) as u16;
+
+    let mut device = Vec::new();
+    device.extend_from_slice(&rec_len.to_le_bytes());
+    device.push(desc_count);
+    device.extend_from_slice(&0u32.to_le_bytes()); // option flags
+    device.push(1u8); // set version string type (utf-8)
+    device.push(set_ver.len() as u8);
+    device.extend_from_slice(&pkg_data_len.to_le_bytes());
+    device.extend_from_slice(&rec_body);
+
+    // Header bytes following the 19-byte init region, excluding the trailing
+    // 4-byte checksum. `offsets` carries each component's absolute file offset.
+    let build_pre = |offsets: &[usize]| -> Vec<u8> {
+        let mut pre = Vec::new();
+        pre.extend_from_slice(&[0u8; 13]); // release date/time
+        pre.extend_from_slice(&(ncomp as u16).to_le_bytes()); // bitmap length (bits)
+        // package version string (type, length, data)
+        pre.push(1u8);
+        pre.push(4u8);
+        pre.extend_from_slice(b"0000");
+        // device id record area
+        pre.push(1u8); // device count
+        pre.extend_from_slice(&device);
+        // downstream device id record area (1.1.x): none
+        pre.push(0u8);
+        // component image information area
+        pre.extend_from_slice(&(ncomp as u16).to_le_bytes());
+        for (i, c) in components.iter().enumerate() {
+            pre.extend_from_slice(&0x000au16.to_le_bytes()); // classification: firmware
+            pre.extend_from_slice(&(i as u16).to_le_bytes()); // identifier
+            pre.extend_from_slice(&0u32.to_le_bytes()); // comparison stamp
+            pre.extend_from_slice(&0u16.to_le_bytes()); // options
+            pre.extend_from_slice(&0u16.to_le_bytes()); // activation method
+            pre.extend_from_slice(&(offsets[i] as u32).to_le_bytes());
+            pre.extend_from_slice(&(c.len() as u32).to_le_bytes());
+            // component version string (type, length, data)
+            pre.push(1u8);
+            pre.push(4u8);
+            pre.extend_from_slice(b"0000");
+        }
+        pre
+    };
+
+    const HDR_INIT_SIZE: usize = 16 + 1 + 2;
+
+    // First pass with placeholder offsets to learn the header size, which is
+    // independent of the (fixed-size) offset field values.
+    let pre_len = build_pre(&vec![0usize; ncomp]).len();
+    let hdr_size = HDR_INIT_SIZE + pre_len + 4;
+
+    // Second pass: real offsets point past the header into the payload area.
+    let mut offsets = vec![0usize; ncomp];
+    let mut cum = hdr_size;
+    for (i, c) in components.iter().enumerate() {
+        offsets[i] = cum;
+        cum += c.len();
+    }
+    let pre = build_pre(&offsets);
+
+    let mut header = Vec::new();
+    header.extend_from_slice(PKG_UUID_1_1_X.as_bytes());
+    header.push(1u8); // header format revision
+    header.extend_from_slice(&(hdr_size as u16).to_le_bytes());
+    header.extend_from_slice(&pre);
+
+    let crc32 = crc::Crc::<u32>::new(&crc::CRC_32_ISO_HDLC);
+    let checksum = crc32.checksum(&header);
+    header.extend_from_slice(&checksum.to_le_bytes());
+    assert_eq!(header.len(), hdr_size);
+
+    let mut file = header;
+    for c in components {
+        file.extend_from_slice(c);
+    }
+    file
+}

pldm-fw/src/ua.rs

@@ -180,9 +180,13 @@ pub fn request_update(
     check_fd_state(comm, PldmFDState::Idle)?;
 
     let sz = XFER_SIZE as u32;
+    let num_components: u16 =
+        update.components.len().try_into().map_err(|_| {
+            PldmUpdateError::new_update("too many components".into())
+        })?;
     let mut data = vec![];
     data.extend_from_slice(&sz.to_le_bytes());
-    data.extend_from_slice(&1u16.to_le_bytes()); // NumberOfComponents
+    data.extend_from_slice(&num_components.to_le_bytes()); // NumberOfComponents
     data.extend_from_slice(&1u8.to_le_bytes()); // MaximumOutstandingTransferRequests
     data.extend_from_slice(&0u16.to_le_bytes()); // PackageDataLength
     update.package.version.write_utf8_bytes(&mut data);
@@ -582,3 +586,150 @@ fn check_fd_state(
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{request_update, Update};
+    use crate::{
+        DeviceCapabilities, Descriptor, DescriptorString, DeviceIdentifiers,
+        FirmwareParameters, GetStatusResponse, PldmFDState, PLDM_TYPE_FW,
+    };
+    use mctp::{Eid, MsgIC, MsgType, ReqChannel, MCTP_TYPE_PLDM};
+    use std::cell::RefCell;
+    use std::collections::VecDeque;
+    use std::rc::Rc;
+
+    // --- Mock MCTP transport ---------------------------------------------
+    //
+    // request_update only drives the UA-initiated `ReqChannel` (`comm`):
+    // a GetStatus precondition followed by RequestUpdate. The mock replays
+    // scripted FD responses and records everything the UA sends so the test
+    // can assert on the wire format.
+
+    #[derive(Default)]
+    struct CommState {
+        responses: VecDeque<Vec<u8>>,
+        sent: Vec<Vec<u8>>,
+    }
+
+    #[derive(Clone, Default)]
+    struct MockComm {
+        st: Rc<RefCell<CommState>>,
+    }
+
+    impl MockComm {
+        fn new(responses: Vec<Vec<u8>>) -> Self {
+            Self {
+                st: Rc::new(RefCell::new(CommState {
+                    responses: responses.into(),
+                    sent: Vec::new(),
+                })),
+            }
+        }
+
+        fn sent(&self) -> Vec<Vec<u8>> {
+            self.st.borrow().sent.clone()
+        }
+    }
+
+    impl ReqChannel for MockComm {
+        fn send_vectored(
+            &mut self,
+            _typ: MsgType,
+            _ic: MsgIC,
+            bufs: &[&[u8]],
+        ) -> mctp::Result<()> {
+            let mut msg = Vec::new();
+            for b in bufs {
+                msg.extend_from_slice(b);
+            }
+            self.st.borrow_mut().sent.push(msg);
+            Ok(())
+        }
+
+        fn recv<'f>(
+            &mut self,
+            buf: &'f mut [u8],
+        ) -> mctp::Result<(MsgType, MsgIC, &'f mut [u8])> {
+            let resp = self
+                .st
+                .borrow_mut()
+                .responses
+                .pop_front()
+                .ok_or(mctp::Error::RxFailure)?;
+            buf[..resp.len()].copy_from_slice(&resp);
+            Ok((MCTP_TYPE_PLDM, MsgIC(false), &mut buf[..resp.len()]))
+        }
+
+        fn remote_eid(&self) -> Eid {
+            Eid(8)
+        }
+    }
+
+    // --- frame helpers ---------------------------------------------------
+
+    /// A PLDM response frame as returned by the FD over `comm`.
+    fn resp_frame(cmd: u8, cc: u8, data: &[u8]) -> Vec<u8> {
+        let mut v = vec![0u8, PLDM_TYPE_FW, cmd, cc];
+        v.extend_from_slice(data);
+        v
+    }
+
+    fn status_data(state: PldmFDState) -> Vec<u8> {
+        let s = GetStatusResponse {
+            current_state: state,
+            previous_state: PldmFDState::Idle,
+            aux_state: 0,
+            aux_state_status: 0,
+            progress_percent: 0,
+            reason_code: 0,
+            update_option_flags_enabled: 0,
+        };
+        let mut b = [0u8; 16];
+        let l = s.write_buf(&mut b).unwrap();
+        b[..l].to_vec()
+    }
+
+    fn make_update(vid: u16, components: &[&[u8]]) -> Update {
+        let bytes = crate::pkg::build_v11_package(vid, components);
+        let pkg =
+            crate::pkg::Package::parse(crate::pkg::temp_file_with(&bytes))
+                .unwrap();
+        let dev = DeviceIdentifiers {
+            ids: vec![Descriptor::PciVid(vid)],
+        };
+        let fwp = FirmwareParameters {
+            caps: DeviceCapabilities::from_u32(0),
+            components: Vec::new().into(),
+            active: DescriptorString::empty(),
+            pending: DescriptorString::empty(),
+        };
+        Update::new(&dev, &fwp, pkg, None, None, vec![]).unwrap()
+    }
+
+    // --- RequestUpdate ---------------------------------------------------
+
+    #[test]
+    fn request_update_reports_actual_component_count() {
+        let img: &[u8] = &[0u8; 64];
+        let update = make_update(0xabcd, &[img, img]);
+
+        let mut comm = MockComm::new(vec![
+            resp_frame(0x1b, 0, &status_data(PldmFDState::Idle)),
+            // RequestUpdateResponse: FirmwareDeviceMetaDataLength + flag
+            resp_frame(0x10, 0, &[0, 0, 0]),
+        ]);
+
+        request_update(&mut comm, &update).unwrap();
+
+        let sent = comm.sent();
+        assert_eq!(sent.len(), 2);
+        // sent[1] is RequestUpdate: [0x80, type, 0x10, <payload>]
+        let ru = &sent[1];
+        assert_eq!(ru[2], 0x10);
+        let data = &ru[3..];
+        // payload: MaxTransferSize(4), NumberOfComponents(2), ...
+        let num_components = u16::from_le_bytes([data[4], data[5]]);
+        assert_eq!(num_components, 2);
+    }
+}