]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
msg/async: fix unnecessary 4 kB allocation in secure mode. 33816/head
authorRadoslaw Zarzynski <rzarzyns@redhat.com>
Sun, 8 Mar 2020 23:25:46 +0000 (00:25 +0100)
committerRadoslaw Zarzynski <rzarzyns@redhat.com>
Mon, 9 Mar 2020 10:52:14 +0000 (11:52 +0100)
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>
src/msg/async/frames_v2.h

index ddc42a489cf04af4eb94283985063f7ea3233f60..87743c7038450cd31ad0db129d80e7f01389bf89 100644 (file)
@@ -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: