1. 19 Mar, 2015 1 commit
    • mancha security's avatar
      lib: memzero_explicit: use barrier instead of OPTIMIZER_HIDE_VAR · 0b053c95
      mancha security authored
      OPTIMIZER_HIDE_VAR(), as defined when using gcc, is insufficient to
      ensure protection from dead store optimization.
      
      For the random driver and crypto drivers, calls are emitted ...
      
        $ gdb vmlinux
        (gdb) disassemble memzero_explicit
        Dump of assembler code for function memzero_explicit:
          0xffffffff813a18b0 <+0>:	push   %rbp
          0xffffffff813a18b1 <+1>:	mov    %rsi,%rdx
          0xffffffff813a18b4 <+4>:	xor    %esi,%esi
          0xffffffff813a18b6 <+6>:	mov    %rsp,%rbp
          0xffffffff813a18b9 <+9>:	callq  0xffffffff813a7120 <memset>
          0xffffffff813a18be <+14>:	pop    %rbp
          0xffffffff813a18bf <+15>:	retq
        End of assembler dump.
      
        (gdb) disassemble extract_entropy
        [...]
          0xffffffff814a5009 <+313>:	mov    %r12,%rdi
          0xffffffff814a500c <+316>:	mov    $0xa,%esi
          0xffffffff814a5011 <+321>:	callq  0xffffffff813a18b0 <memzero_explicit>
          0xffffffff814a5016 <+326>:	mov    -0x48(%rbp),%rax
        [...]
      
      ... but in case in future we might use facilities such as LTO, then
      OPTIMIZER_HIDE_VAR() is not sufficient to protect gcc from a possible
      eviction of the memset(). We have to use a compiler barrier instead.
      
      Minimal test example when we assume memzero_explicit() would *not* be
      a call, but would have been *inlined* instead:
      
        static inline void memzero_explicit(void *s, size_t count)
        {
          memset(s, 0, count);
          <foo>
        }
      
        int main(void)
        {
          char buff[20];
      
          snprintf(buff, sizeof(buff) - 1, "test");
          printf("%s", buff);
      
          memzero_explicit(buff, sizeof(buff));
          return 0;
        }
      
      With <foo> := OPTIMIZER_HIDE_VAR():
      
        (gdb) disassemble main
        Dump of assembler code for function main:
        [...]
         0x0000000000400464 <+36>:	callq  0x400410 <printf@plt>
         0x0000000000400469 <+41>:	xor    %eax,%eax
         0x000000000040046b <+43>:	add    $0x28,%rsp
         0x000000000040046f <+47>:	retq
        End of assembler dump.
      
      With <foo> := barrier():
      
        (gdb) disassemble main
        Dump of assembler code for function main:
        [...]
         0x0000000000400464 <+36>:	callq  0x400410 <printf@plt>
         0x0000000000400469 <+41>:	movq   $0x0,(%rsp)
         0x0000000000400471 <+49>:	movq   $0x0,0x8(%rsp)
         0x000000000040047a <+58>:	movl   $0x0,0x10(%rsp)
         0x0000000000400482 <+66>:	xor    %eax,%eax
         0x0000000000400484 <+68>:	add    $0x28,%rsp
         0x0000000000400488 <+72>:	retq
        End of assembler dump.
      
      As can be seen, movq, movq, movl are being emitted inlined
      via memset().
      
      Reference: http://thread.gmane.org/gmane.linux.kernel.cryptoapi/13764/
      Fixes: d4c5efdb ("random: add and use memzero_explicit() for clearing data")
      Cc: Theodore Ts'o <tytso@mit.edu>
      Signed-off-by: default avatarmancha security <mancha1@zoho.com>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: default avatarHannes Frederic Sowa <hannes@stressinduktion.org>
      Acked-by: default avatarStephan Mueller <smueller@chronox.de>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      0b053c95
  2. 18 Mar, 2015 1 commit
  3. 17 Mar, 2015 3 commits
    • Herbert Xu's avatar
      linux-next: build failure after merge of the crypto tree · 7094e8ea
      Herbert Xu authored
      crypto: img-hash - Add missing semicolon to fix build error
      
      There is a missing semicolon after MODULE_DEVICE_TABLE.
      Reported-by: default avatarStephen Rothwell <sfr@canb.auug.org.au>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      7094e8ea
    • Andre Wolokita's avatar
      hwrng: omap - Change RNG_CONFIG_REG to RNG_CONTROL_REG in init · 656d7e7e
      Andre Wolokita authored
      omap4_rng_init() checks bit 10 of the RNG_CONFIG_REG to determine whether
      the RNG is already running before performing any initiliasation. This is not
      the correct register to check, as the enable bit is in RNG_CONFIG_CONTROL.
      Read from RNG_CONTROL_REG instead.
      Signed-off-by: default avatarAndre Wolokita <Andre.Wolokita@analog.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      656d7e7e
    • Andre Wolokita's avatar
      hwrng: omap - Change RNG_CONFIG_REG to RNG_CONTROL_REG when checking and disabling TRNG · 1a5addfe
      Andre Wolokita authored
      In omap4_rng_init(), a check of bit 10 of the RNG_CONFIG_REG is done to determine
      whether the RNG is running. This is suspicious firstly due to the use of
      RNG_CONTROL_ENABLE_TRNG_MASK and secondly because the same mask is written to
      RNG_CONTROL_REG after configuration of the FROs. Similar suspicious logic is
      repeated in omap4_rng_cleanup() when RNG_CONTROL_REG masked with
      RNG_CONTROL_ENABLE_TRNG_MASK is read, the same mask bit is cleared, and then
      written to RNG_CONFIG_REG. Unless the TRNG is enabled with one bit in RNG_CONTROL
      and disabled with another in RNG_CONFIG and these bits are mirrored in some way,
      I believe that the TRNG is not really shutting off.
      
      Apart from the strange logic, I have reason to suspect that the OMAP4 related
      code in this driver is driving an Inside Secure IP hardware RNG and strongly
      suspect that bit 10 of RNG_CONFIG_REG is one of the bits configuring the
      sampling rate of the FROs. This option is by default set to 0 and is not being
      set anywhere in omap-rng.c. Reading this bit during omap4_rng_init() will
      always return 0. It will remain 0 because ~(value of TRNG_MASK in control) will
      always be 0, because the TRNG is never shut off. This is of course presuming
      that the OMAP4 features the Inside Secure IP.
      
      I'm interested in knowing what the guys at TI think about this, as only they
      can confirm or deny the detailed structure of these registers.
      Signed-off-by: default avatarAndre Wolokita <Andre.Wolokita@analog.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      1a5addfe
  4. 16 Mar, 2015 10 commits
  5. 13 Mar, 2015 2 commits
  6. 12 Mar, 2015 7 commits
  7. 11 Mar, 2015 6 commits
    • Dmitry Torokhov's avatar
      crypto: amcc - remove incorrect __init/__exit markups · 1eb8a1b3
      Dmitry Torokhov authored
      Even if bus is not hot-pluggable, the devices can be bound and unbound
      from the driver via sysfs, so we should not be using __init/__exit
      annotations on probe() and remove() methods. The only exception is
      drivers registered with platform_driver_probe() which specifically
      disables sysfs bind/unbind attributes.
      Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      1eb8a1b3
    • Dmitry Torokhov's avatar
      crypto: qat - remove incorrect __exit markup · 83ce01d2
      Dmitry Torokhov authored
      PCI bus is hot-pluggable, and even if it wasn't one can still unbind the
      device from driver via sysfs, so we should not make driver's remove
      method as __exit.
      Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      83ce01d2
    • Dmitry Torokhov's avatar
      hwrng: pseries - remove incorrect __init/__exit markups · 257bedd4
      Dmitry Torokhov authored
      Even if bus is not hot-pluggable, the devices can be unbound from the
      driver via sysfs, so we should not be using __exit annotations on
      remove() methods. The only exception is drivers registered with
      platform_driver_probe() which specifically disables sysfs bind/unbind
      attributes.
      
      Similarly probe() methods should not be marked __init unless
      platform_driver_probe() is used.
      Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      257bedd4
    • Dmitry Torokhov's avatar
      hwrng: octeon - remove incorrect __exit markups · 87094a04
      Dmitry Torokhov authored
      Even if bus is not hot-pluggable, the devices can be unbound from the
      driver via sysfs, so we should not be using __exit annotations on
      remove() methods. The only exception is drivers registered with
      platform_driver_probe() which specifically disables sysfs bind/unbind
      attributes
      Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      87094a04
    • Dmitry Torokhov's avatar
      hwrng: omap - remove incorrect __exit markups · 1ee9b5e4
      Dmitry Torokhov authored
      Even if bus is not hot-pluggable, the devices can be unbound from the
      driver via sysfs, so we should not be using __exit annotations on
      remove() methods. The only exception is drivers registered with
      platform_driver_probe() which specifically disables sysfs bind/unbind
      attributes.
      Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      1ee9b5e4
    • Horia Geant?'s avatar
      crypto: tcrypt - fix uninit sg entries in test_acipher_speed · 007ee8de
      Horia Geant? authored
      Commit 5be4d4c9 ("crypto: replace scatterwalk_sg_next with sg_next")
      did not consider the fact that scatterwalk_sg_next() was looking at
      sg entry length, while sg_next() looks at the "chained" sg bit.
      
      This should have no effect in theory. However in practice, there are
      cases where the sg table is initialized to a number of entries and
      some of them are not properly configured. While scatterwalk_sg_next()
      would have returned NULL (since sg length = 0 and sg page_link = 0),
      sg_next() happily returns the next unconfigured sg entry.
      
      insmod tcrypt.ko mode=500 sec=1
      
      testing speed of async cbc(aes) (cbc-aes-talitos) encryption
      test 0 (128 bit key, 16 byte blocks):
      Unable to handle kernel paging request for data at address 0x00000000
      Faulting instruction address: 0xc00d79e4
      Oops: Kernel access of bad area, sig: 11 [#1]
      SMP NR_CPUS=8 P1022 DS
      Modules linked in: tcrypt(+) talitos
      CPU: 0 PID: 2670 Comm: insmod Not tainted 4.0.0-rc1-QorIQ-SDK-V1.6+g904f1ca82209 #1
      task: e8de3200 ti: e70bc000 task.ti: e70bc000
      NIP: c00d79e4 LR: f92d223c CTR: c00d79c8
      REGS: e70bda00 TRAP: 0300   Not tainted  (4.0.0-rc1-QorIQ-SDK-V1.6+g904f1ca82209)
      MSR: 00029000 <CE,EE,ME>  CR: 84428f22  XER: 00000000
      DEAR: 00000000 ESR: 00000000
      GPR00: f92d223c e70bdab0 e8de3200 00000000 e70bdbb8 00000001 00000000 00000000
      GPR08: 00000000 00000000 c08b0380 27282010 c00d79c8 1003a634 00000000 e70bdf1c
      GPR16: e70bdef0 00000020 00000000 c08c0000 00000010 00000000 e70bdbb8 00000010
      GPR24: e976d3a8 00000010 00000000 e70bdbd8 e8961010 00000001 c086e560 00000000
      NIP [c00d79e4] page_address+0x1c/0x110
      LR [f92d223c] talitos_map_sg+0x130/0x184 [talitos]
      Call Trace:
      [e70bdab0] [00000010] 0x10 (unreliable)
      [e70bdad0] [f92d223c] talitos_map_sg+0x130/0x184 [talitos]
      [e70bdb00] [f92d30d8] common_nonsnoop.constprop.13+0xc0/0x304 [talitos]
      [e70bdb30] [f933fd90] test_acipher_speed+0x434/0x7dc [tcrypt]
      [e70bdcc0] [f934318c] do_test+0x2478/0x306c [tcrypt]
      [e70bdd80] [f11fe058] tcrypt_mod_init+0x58/0x100 [tcrypt]
      [e70bdda0] [c0002354] do_one_initcall+0x90/0x1f4
      [e70bde10] [c061fe00] do_init_module+0x60/0x1ac
      [e70bde30] [c00a79f0] load_module+0x185c/0x1f88
      [e70bdee0] [c00a82b0] SyS_finit_module+0x7c/0x98
      [e70bdf40] [c000e8b0] ret_from_syscall+0x0/0x3c
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      007ee8de
  8. 10 Mar, 2015 7 commits
  9. 09 Mar, 2015 3 commits