[PATCH] perf(message): avoid buffer copy during command parsing

[PATCH] perf(message): avoid buffer copy during command parsing

[Jarkko Sakkinen]

`tpm_parse_command` copies handles and parameters to a fixed-size temporary
4 KiB buffer on the stack before invoking a command parser.

Introduce `TpmParseCommandBody` trait implemented by `tpm_struct!`,
providing accessors for handles and parameters in the original command
buffer, thus zeroing out the large stack allocation.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
I plan to backport this also to 0.10.x branch, given signficancy.
 src/macro/mod.rs     |  7 +++++--
 src/macro/struct.rs  | 20 +++++++++++++++-----
 src/message/mod.rs   | 14 +++++++++++++-
 src/message/parse.rs | 15 +++------------
 4 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/src/macro/mod.rs b/src/macro/mod.rs
index 2ca6a15..782d3f7 100644
--- a/src/macro/mod.rs
+++ b/src/macro/mod.rs
@@ -150,7 +150,10 @@ macro_rules! tpm_dispatch {
                     <$value as $crate::message::TpmHeader>::NO_SESSIONS,
                     <$value as $crate::message::TpmHeader>::WITH_SESSIONS,
                     <$value as $crate::message::TpmHeader>::HANDLES,
-                    |buf| <$value>::parse(buf).map(|(c, r)| (TpmCommandBody::$name(c), r)),
+                    |handles, params| {
+                        <$value as $crate::message::TpmCommandBodyParse>::parse_body(handles, params)
+                            .map(|(c, r)| (TpmCommandBody::$name(c), r))
+                    },
                 )
             };
         }
@@ -199,7 +202,7 @@ macro_rules! tpm_dispatch {
             )*
         }
 
-        pub type TpmCommandParser = for<'a> fn(&'a [u8]) -> $crate::TpmResult<(TpmCommandBody, &'a [u8])>;
+        pub type TpmCommandParser = for<'a> fn(&'a [u8], &'a [u8]) -> $crate::TpmResult<(TpmCommandBody, &'a [u8])>;
         pub type TpmResponseParser = for<'a> fn(&'a [u8]) -> $crate::TpmResult<(TpmResponseBody, &'a [u8])>;
 
         pub(crate) static PARSE_COMMAND_MAP: &[($crate::data::TpmCc, bool, bool, usize, TpmCommandParser)] =
diff --git a/src/macro/struct.rs b/src/macro/struct.rs
index d9bc593..01e2cfb 100644
--- a/src/macro/struct.rs
+++ b/src/macro/struct.rs
@@ -55,18 +55,28 @@ macro_rules! tpm_struct {
             }
         }
 
-        impl $crate::TpmParse for $name {
+        impl $crate::message::TpmCommandBodyParse for $name {
             #[allow(unused_mut)]
-            fn parse(buf: &[u8]) -> $crate::TpmResult<(Self, &[u8])> {
-                let mut cursor = buf;
+            fn parse_body<'a>(
+                handles: &'a [u8],
+                params: &'a [u8],
+            ) -> $crate::TpmResult<(Self, &'a [u8])> {
+                let mut cursor = handles;
                 $(
-                    let ($handle_field, tail) = <$handle_type>::parse(cursor)?;
+                    let ($handle_field, tail) = <$handle_type as $crate::TpmParse>::parse(cursor)?;
                     cursor = tail;
                 )*
+
+                if !cursor.is_empty() {
+                    return Err($crate::TpmErrorKind::TrailingData);
+                }
+
+                let mut cursor = params;
                 $(
-                    let ($param_field, tail) = <$param_type>::parse(cursor)?;
+                    let ($param_field, tail) = <$param_type as $crate::TpmParse>::parse(cursor)?;
                     cursor = tail;
                 )*
+
                 Ok((
                     Self {
                         $($handle_field,)*
diff --git a/src/message/mod.rs b/src/message/mod.rs
index aff0933..eb0d3f0 100644
--- a/src/message/mod.rs
+++ b/src/message/mod.rs
@@ -52,7 +52,7 @@ pub type TpmAuthCommands = TpmList<data::TpmsAuthCommand, MAX_SESSIONS>;
 /// A fixed-capacity list for response authorization sessions.
 pub type TpmAuthResponses = TpmList<data::TpmsAuthResponse, MAX_SESSIONS>;
 /// A trait for TPM commands and responses that provides header information.
-pub trait TpmHeader: TpmBuild + TpmParse + Debug {
+pub trait TpmHeader: TpmBuild + Debug {
     const COMMAND: data::TpmCc;
     const NO_SESSIONS: bool;
     const WITH_SESSIONS: bool;
@@ -77,6 +77,18 @@ pub trait TpmHeaderCommand: TpmHeader {
     fn build_parameters(&self, writer: &mut TpmWriter) -> TpmResult<()>;
 }
 
+/// Parses a command body from the slices point out to the handle area and
+/// parameter area of the original buffer.
+pub(crate) trait TpmCommandBodyParse: Sized {
+    /// Parses the command body from the handle and parameter area.
+    ///
+    /// # Errors
+    ///
+    /// * `TpmErrorKind::ParseCapacity` if the capacity limit is exceeded
+    /// * `TpmErrorKind::ParseUnderflow` if the parser runs out of bytes
+    fn parse_body<'a>(handles: &'a [u8], params: &'a [u8]) -> TpmResult<(Self, &'a [u8])>;
+}
+
 pub const TPM_HEADER_SIZE: usize = 10;
 
 tpm_dispatch! {
diff --git a/src/message/parse.rs b/src/message/parse.rs
index cc63802..f55d7a8 100644
--- a/src/message/parse.rs
+++ b/src/message/parse.rs
@@ -8,7 +8,7 @@ use super::{
 };
 use crate::{
     data::{TpmCc, TpmRc, TpmSt, TpmsAuthCommand, TpmsAuthResponse},
-    TpmErrorKind, TpmNotDiscriminant, TpmParse, TpmResult, TPM_MAX_COMMAND_SIZE,
+    TpmErrorKind, TpmNotDiscriminant, TpmParse, TpmResult,
 };
 use core::{convert::TryFrom, mem::size_of};
 
@@ -94,18 +94,9 @@ pub fn tpm_parse_command(buf: &[u8]) -> TpmResult<(TpmHandles, TpmCommandBody, T
         after_handles
     };
 
-    let mut temp_body_buf = [0u8; TPM_MAX_COMMAND_SIZE];
-    let full_body_len = handle_area.len() + param_area.len();
-    if full_body_len > temp_body_buf.len() {
-        return Err(TpmErrorKind::ParseCapacity);
-    }
-    let full_body_for_parser = &mut temp_body_buf[..full_body_len];
-    full_body_for_parser[..handle_area.len()].copy_from_slice(handle_area);
-    full_body_for_parser[handle_area.len()..].copy_from_slice(param_area);
-
-    let (command_data, remainder) = (dispatch.4)(full_body_for_parser)?;
+    let (command_data, param_remainder) = (dispatch.4)(handle_area, param_area)?;
 
-    if !remainder.is_empty() {
+    if !param_remainder.is_empty() {
         return Err(TpmErrorKind::TrailingData);
     }
 
-- 
2.39.5

Re: [PATCH] perf(message): avoid buffer copy during command parsing

[Jarkko Sakkinen]

On Sun, Aug 31, 2025 at 06:01:45PM +0300, Jarkko Sakkinen wrote:
> `tpm_parse_command` copies handles and parameters to a fixed-size temporary
> 4 KiB buffer on the stack before invoking a command parser.
> 
> Introduce `TpmParseCommandBody` trait implemented by `tpm_struct!`,
> providing accessors for handles and parameters in the original command
> buffer, thus zeroing out the large stack allocation.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> I plan to backport this also to 0.10.x branch, given signficancy.

This is something left out of initial stretch, which I've had in my mind
for some time. I postponed this change because stability was a priority
over performance at the time.

For the sake of symmetry I want to also rename `TpmHeaderCommand` as
`TpmCommandBuild`, make `TpmHeader` decoupled from that trait, and
finally change the constraint in `tpm_build_command` from `C:
TpmHeaderCommand` to `C: TpmHeader + TpmCommandBuild`.

This patch does not include that style change, given that it is first of
all logically distinct and further cannot be backported to the 0.10.x
branch.

BR, Jarkko

Re: [PATCH] perf(message): avoid buffer copy during command parsing

[Jarkko Sakkinen]

On Sun, Aug 31, 2025 at 06:19:22PM +0300, Jarkko Sakkinen wrote:
> On Sun, Aug 31, 2025 at 06:01:45PM +0300, Jarkko Sakkinen wrote:
> > `tpm_parse_command` copies handles and parameters to a fixed-size temporary
> > 4 KiB buffer on the stack before invoking a command parser.
> > 
> > Introduce `TpmParseCommandBody` trait implemented by `tpm_struct!`,
> > providing accessors for handles and parameters in the original command
> > buffer, thus zeroing out the large stack allocation.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > I plan to backport this also to 0.10.x branch, given signficancy.
> 
> This is something left out of initial stretch, which I've had in my mind
> for some time. I postponed this change because stability was a priority
> over performance at the time.
> 
> For the sake of symmetry I want to also rename `TpmHeaderCommand` as
> `TpmCommandBuild`, make `TpmHeader` decoupled from that trait, and
> finally change the constraint in `tpm_build_command` from `C:
> TpmHeaderCommand` to `C: TpmHeader + TpmCommandBuild`.
> 
> This patch does not include that style change, given that it is first of
> all logically distinct and further cannot be backported to the 0.10.x
> branch.

As this is just backlog and dead obvious fix, I'll also apply this to
the main branch and backport to the 0.11.x branch.

BR, Jarkko