1. 12 Mar, 2020 1 commit
  2. 06 Mar, 2020 28 commits
  3. 28 Feb, 2020 8 commits
  4. 22 Feb, 2020 3 commits
    • Al Viro's avatar
      crypto: chelsio - Endianess bug in create_authenc_wr · ff462ddf
      Al Viro authored
      kctx_len = (ntohl(KEY_CONTEXT_CTX_LEN_V(aeadctx->key_ctx_hdr)) << 4)
                      - sizeof(chcr_req->key_ctx);
      can't possibly be endian-safe.  Look: ->key_ctx_hdr is __be32.  And
      KEY_CONTEXT_CTX_LEN_V is "shift up by 24 bits".  On little-endian hosts it
      sees
      	b0 b1 b2 b3
      in memory, inteprets that into b0 + (b1 << 8) + (b2 << 16) + (b3 << 24),
      shifts up by 24, resulting in b0 << 24, does ntohl (byteswap on l-e),
      gets b0 and shifts that up by 4.  So we get b0 * 16 - sizeof(...).
      
      Sounds reasonable, but on b-e we get
      b3 + (b2 << 8) + (b1 << 16) + (b0 << 24), shift up by 24,
      yielding b3 << 24, do ntohl (no-op on b-e) and then shift up by 4.
      Resulting in b3 << 28 - sizeof(...), i.e. slightly under b3 * 256M.
      
      Then we increase it some more and pass to alloc_skb() as size.
      Somehow I doubt that we really want a quarter-gigabyte skb allocation
      here...
      
      Note that when you are building those values in
      #define  FILL_KEY_CTX_HDR(ck_size, mk_size, d_ck, opad, ctx_len) \
                      htonl(KEY_CONTEXT_VALID_V(1) | \
                            KEY_CONTEXT_CK_SIZE_V((ck_size)) | \
                            KEY_CONTEXT_MK_SIZE_V(mk_size) | \
                            KEY_CONTEXT_DUAL_CK_V((d_ck)) | \
                            KEY_CONTEXT_OPAD_PRESENT_V((opad)) | \
                            KEY_CONTEXT_SALT_PRESENT_V(1) | \
                            KEY_CONTEXT_CTX_LEN_V((ctx_len)))
      ctx_len ends up in the first octet (i.e. b0 in the above), which
      matches the current behaviour on l-e.  If that's the intent, this
      thing should've been
              kctx_len = (KEY_CONTEXT_CTX_LEN_G(ntohl(aeadctx->key_ctx_hdr)) << 4)
                      - sizeof(chcr_req->key_ctx);
      instead - fetch after ntohl() we get (b0 << 24) + (b1 << 16) + (b2 << 8) + b3,
      shift it down by 24 (b0), resuling in b0 * 16 - sizeof(...) both on l-e and
      on b-e.
      
      PS: when sparse warns you about endianness problems, it might be worth checking
      if there really is something wrong.  And I don't mean "slap __force cast on it"...
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      ff462ddf
    • Gustavo A. R. Silva's avatar
      crypto: s5p-sss - Replace zero-length array with flexible-array member · a4a70fa9
      Gustavo A. R. Silva authored
      The current codebase makes use of the zero-length array language
      extension to the C90 standard, but the preferred mechanism to declare
      variable-length types such as these ones is a flexible array member[1][2],
      introduced in C99:
      
      struct foo {
              int stuff;
              struct boo array[];
      };
      
      By making use of the mechanism above, we will get a compiler warning
      in case the flexible array does not occur last in the structure, which
      will help us prevent some kind of undefined behavior bugs from being
      inadvertently introduced[3] to the codebase from now on.
      
      Also, notice that, dynamic memory allocations won't be affected by
      this change:
      
      "Flexible array members have incomplete type, and so the sizeof operator
      may not be applied. As a quirk of the original implementation of
      zero-length arrays, sizeof evaluates to zero."[1]
      
      This issue was found with the help of Coccinelle.
      
      [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
      [2] https://github.com/KSPP/linux/issues/21
      [3] commit 76497732 ("cxgb3/l2t: Fix undefined behaviour")
      Signed-off-by: default avatarGustavo A. R. Silva <gustavo@embeddedor.com>
      Acked-by: default avatarKamil Konieczny <k.konieczny@samsung.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      a4a70fa9
    • Gustavo A. R. Silva's avatar
      crypto: img-hash - Replace zero-length array with flexible-array member · e44362ab
      Gustavo A. R. Silva authored
      The current codebase makes use of the zero-length array language
      extension to the C90 standard, but the preferred mechanism to declare
      variable-length types such as these ones is a flexible array member[1][2],
      introduced in C99:
      
      struct foo {
              int stuff;
              struct boo array[];
      };
      
      By making use of the mechanism above, we will get a compiler warning
      in case the flexible array does not occur last in the structure, which
      will help us prevent some kind of undefined behavior bugs from being
      inadvertently introduced[3] to the codebase from now on.
      
      Also, notice that, dynamic memory allocations won't be affected by
      this change:
      
      "Flexible array members have incomplete type, and so the sizeof operator
      may not be applied. As a quirk of the original implementation of
      zero-length arrays, sizeof evaluates to zero."[1]
      
      This issue was found with the help of Coccinelle.
      
      [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
      [2] https://github.com/KSPP/linux/issues/21
      [3] commit 76497732 ("cxgb3/l2t: Fix undefined behaviour")
      Signed-off-by: default avatarGustavo A. R. Silva <gustavo@embeddedor.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      e44362ab