[bug report] selftests/tls: add a test for fragmented messages

[bug report] selftests/tls: add a test for fragmented messages

[Dan Carpenter]

Hello self test developers,

I feel like I reported this years ago but I forget what is going on
here?

The patch 65190f77424d: "selftests/tls: add a test for fragmented
messages" from Nov 27, 2019, leads to the following static checker
warning:

	tools/testing/selftests/net/tls.c:292 tls_sendmsg_fragmented()
	warn: curly braces intended?

tools/testing/selftests/net/tls.c
   299  TEST_F(tls, sendmsg_large)
   300  {
   301          void *mem = malloc(16384);
   302          size_t send_len = 16384;
   303          size_t sends = 128;
   304          struct msghdr msg;
   305          size_t recvs = 0;
   306          size_t sent = 0;
   307  
   308          memset(&msg, 0, sizeof(struct msghdr));
   309          while (sent++ < sends) {
   310                  struct iovec vec = { (void *)mem, send_len };
   311  
   312                  msg.msg_iov = &vec;
   313                  msg.msg_iovlen = 1;
   314                  EXPECT_EQ(sendmsg(self->cfd, &msg, 0), send_len);
   315          }
   316  
   317          while (recvs++ < sends)
   318                  EXPECT_NE(recv(self->fd, mem, send_len, 0), -1);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is a macro (below).

   319  
   320          free(mem);
   321  }

tools/testing/selftests/kselftest_harness.h
   592  /* Support an optional handler after and ASSERT_* or EXPECT_*.  The approach is
   593   * not thread-safe, but it should be fine in most sane test scenarios.
   594   *
   595   * Using __bail(), which optionally abort()s, is the easiest way to early
   596   * return while still providing an optional block to the API consumer.
   597   */
   598  #define OPTIONAL_HANDLER(_assert) \
   599          for (; _metadata->trigger; _metadata->trigger = \
   600                          __bail(_assert, _metadata->no_print, _metadata->step))
   601  
   602  #define __INC_STEP(_metadata) \
   603          if (_metadata->passed && _metadata->step < 255) \
   604                  _metadata->step++;
   605  
   606  #define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \
   607          /* Avoid multiple evaluation of the cases */ \
   608          __typeof__(_expected) __exp = (_expected); \
   609          __typeof__(_seen) __seen = (_seen); \
   610          if (_assert) __INC_STEP(_metadata); \
   611          if (!(__exp _t __seen)) { \
   612                  unsigned long long __exp_print = (uintptr_t)__exp; \
   613                  unsigned long long __seen_print = (uintptr_t)__seen; \
   614                  __TH_LOG("Expected %s (%llu) %s %s (%llu)", \
   615                           _expected_str, __exp_print, #_t, \
   616                           _seen_str, __seen_print); \
   617                  _metadata->passed = 0; \
   618                  /* Ensure the optional handler is triggered */ \
   619                  _metadata->trigger = 1; \
   620          } \
   621  } while (0); OPTIONAL_HANDLER(_assert)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The problem is the OPTIONAL_HANDLER().  Smatch thinks it should be
included inside the do {} while(0) loop.

regards,
dan carpenter

Re: [bug report] selftests/tls: add a test for fragmented messages

[Kees Cook]

On Tue, Jan 07, 2020 at 05:01:06PM +0300, Dan Carpenter wrote:
>    621  } while (0); OPTIONAL_HANDLER(_assert)
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> The problem is the OPTIONAL_HANDLER().  Smatch thinks it should be
> included inside the do {} while(0) loop.

It looks weird, but it is intentional. :)

-- 
Kees Cook