From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Sun, 8 Mar 2020 23:25:46 +0000 (+0100)
Subject: msg/async: fix unnecessary 4 kB allocation in secure mode.
X-Git-Tag: v16.1.0~2614^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=1a975fb3a801ac0191e17c1163f5cf59e59849a7;p=ceph.git

msg/async: fix unnecessary 4 kB allocation in secure mode.

Encryption of outgoing on-wire payload can be performed in
multiple steps -- each segment separately to reflect the multi-
buffer structure of protocol's messages.

For the sake of performance, it is desired to encrypt these
plaintext buffers into single, continuous ciphertext.
At the level of `TxHandler` interface this is facilitated
by `reset_tx_handler()` taking `std::initializer_list` with
the sizes of plainbuffers that are going to be encrypted.
All the implementation does is calling `reserve()` on
the underlying, internal buffer that is used for composing
ciphertext. This provides the opportunity to avoid additional
allocations in both:
  * `authenticated_encrypt_update()`,
  * `authenticated_encrypt_final()`.

However, the recent stack trace obtained from Valgrind showed
that an extra allocation actually happened:

```
  <kind>SyscallParam</kind>
  <what>Syscall param sendmsg(msg.msg_iov[1]) points to uninitialised byte(s)</what>
  ...
  <auxwhat>Address 0xfc805b0 is 0 bytes inside a block of size 4,096 alloc'd</auxwhat>
  <stack>
    ...
    <frame>
      <ip>0xEE7A6D</ip>
      <obj>/usr/bin/ceph-osd</obj>
      <fn>ceph::buffer::v15_2_0::list::refill_append_space(unsigned int)</fn>
      <dir>/usr/src/debug/ceph-15.1.0-1858.gf7e5770.el8.x86_64/src/common</dir>
      <file>buffer.cc</file>
      <line>1324</line>
    </frame>
    <frame>
      <ip>0xEE7EDA</ip>
      <obj>/usr/bin/ceph-osd</obj>
      <fn>ceph::buffer::v15_2_0::list::append_hole(unsigned int)</fn>
      <dir>/usr/src/debug/ceph-15.1.0-1858.gf7e5770.el8.x86_64/src/common</dir>
      <file>buffer.cc</file>
      <line>1444</line>
    </frame>
    <frame>
      <ip>0x10CA5B0</ip>
      <obj>/usr/bin/ceph-osd</obj>
      <fn>ceph::crypto::onwire::AES128GCM_OnWireTxHandler::authenticated_encrypt_final()</fn>
      <dir>/usr/src/debug/ceph-15.1.0-1858.gf7e5770.el8.x86_64/src/msg/async</dir>
      <file>crypto_onwire.cc</file>
      <line>121</line>
    </frame>
  ...
```

The reason is that the size of `epilogue_secure_block_t` has
not been accounted when turning a Message into on-wire payload.
This patch rectifies that.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
---

diff --git a/src/msg/async/frames_v2.h b/src/msg/async/frames_v2.h
index ddc42a489cf0..87743c703845 100644
--- a/src/msg/async/frames_v2.h
+++ b/src/msg/async/frames_v2.h
@@ -232,7 +232,8 @@ private:
     ceph::crypto::onwire::rxtx_t &session_stream_handlers,
     std::index_sequence<Is...>)
   {
-    session_stream_handlers.tx->reset_tx_handler({ segments[Is].length()... });
+    session_stream_handlers.tx->reset_tx_handler({ segments[Is].length()...,
+                                                   sizeof(epilogue_secure_block_t) });
   }
 
 public: