1. 05 Apr, 2016 12 commits
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_raw_from_sgl(): fix nbits calculation · 64c09b0b
      Nicolai Stange authored
      The number of bits, nbits, is calculated in mpi_read_raw_from_sgl() as
      follows:
      
        nbits = nbytes * 8;
      
      Afterwards, the number of leading zero bits of the first byte get
      subtracted:
      
        nbits -= count_leading_zeros(*(u8 *)(sg_virt(sgl) + lzeros));
      
      However, count_leading_zeros() takes an unsigned long and thus,
      the u8 gets promoted to an unsigned long.
      
      Thus, the above doesn't subtract the number of leading zeros in the most
      significant nonzero input byte from nbits, but the number of leading
      zeros of the most significant nonzero input byte promoted to unsigned long,
      i.e. BITS_PER_LONG - 8 too many.
      
      Fix this by subtracting
      
        count_leading_zeros(...) - (BITS_PER_LONG - 8)
      
      from nbits only.
      
      Fixes: 2d4d1eea ("lib/mpi: Add mpi sgl helpers")
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      64c09b0b
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_raw_from_sgl(): purge redundant clearing of nbits · 60e1b74c
      Nicolai Stange authored
      In mpi_read_raw_from_sgl(), unsigned nbits is calculated as follows:
      
        nbits = nbytes * 8;
      
      and redundantly cleared later on if nbytes == 0:
      
        if (nbytes > 0)
          ...
        else
          nbits = 0;
      
      Purge this redundant clearing for the sake of clarity.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      60e1b74c
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_raw_from_sgl(): don't include leading zero SGEs in nbytes · ab1e912e
      Nicolai Stange authored
      At the very beginning of mpi_read_raw_from_sgl(), the leading zeros of
      the input scatterlist are counted:
      
        lzeros = 0;
        for_each_sg(sgl, sg, ents, i) {
          ...
          if (/* sg contains nonzero bytes */)
            break;
      
          /* sg contains nothing but zeros here */
          ents--;
          lzeros = 0;
        }
      
      Later on, the total number of trailing nonzero bytes is calculated by
      subtracting the number of leading zero bytes from the total number of input
      bytes:
      
        nbytes -= lzeros;
      
      However, since lzeros gets reset to zero for each completely zero leading
      sg in the loop above, it doesn't include those.
      
      Besides wasting resources by allocating a too large output buffer,
      this mistake propagates into the calculation of x, the number of
      leading zeros within the most significant output limb:
      
        x = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
      
      What's more, the low order bytes of the output, equal in number to the
      extra bytes in nbytes, are left uninitialized.
      
      Fix this by adjusting nbytes for each completely zero leading scatterlist
      entry.
      
      Fixes: 2d4d1eea ("lib/mpi: Add mpi sgl helpers")
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      ab1e912e
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_raw_from_sgl(): replace len argument by nbytes · b6985389
      Nicolai Stange authored
      Currently, the nbytes local variable is calculated from the len argument
      as follows:
      
        ... mpi_read_raw_from_sgl(..., unsigned int len)
        {
          unsigned nbytes;
          ...
          if (!ents)
            nbytes = 0;
          else
            nbytes = len - lzeros;
          ...
        }
      
      Given that nbytes is derived from len in a trivial way and that the len
      argument is shadowed by a local len variable in several loops, this is just
      confusing.
      
      Rename the len argument to nbytes and get rid of the nbytes local variable.
      Do the nbytes calculation in place.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      b6985389
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_buffer(): fix buffer overflow · 462696fd
      Nicolai Stange authored
      Currently, mpi_read_buffer() writes full limbs to the output buffer
      and moves memory around to purge leading zero limbs afterwards.
      
      However, with
      
        commit 9cbe21d8 ("lib/mpi: only require buffers as big as needed for
                              the integer")
      
      the caller is only required to provide a buffer large enough to hold the
      result without the leading zeros.
      
      This might result in a buffer overflow for small MP numbers with leading
      zeros.
      
      Fix this by coping the result to its final destination within the output
      buffer and not copying the leading zeros at all.
      
      Fixes: 9cbe21d8 ("lib/mpi: only require buffers as big as needed for
                            the integer")
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      462696fd
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_buffer(): replace open coded endian conversion · 90f864e2
      Nicolai Stange authored
      Currently, the endian conversion from CPU order to BE is open coded in
      mpi_read_buffer().
      
      Replace this by the centrally provided cpu_to_be*() macros.
      Copy from the temporary storage on stack to the destination buffer
      by means of memcpy().
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      90f864e2
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_buffer(): optimize skipping of leading zero limbs · f00fa241
      Nicolai Stange authored
      Currently, if the number of leading zeros is greater than fits into a
      complete limb, mpi_read_buffer() skips them by iterating over them
      limb-wise.
      
      Instead of skipping the high order zero limbs within the loop as shown
      above, adjust the copying loop's bounds.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      f00fa241
    • Nicolai Stange's avatar
      lib/mpi: mpi_write_sgl(): replace open coded endian conversion · d7552906
      Nicolai Stange authored
      Currently, the endian conversion from CPU order to BE is open coded in
      mpi_write_sgl().
      
      Replace this by the centrally provided cpu_to_be*() macros.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      d7552906
    • Nicolai Stange's avatar
      lib/mpi: mpi_write_sgl(): fix out-of-bounds stack access · cece762f
      Nicolai Stange authored
      Within the copying loop in mpi_write_sgl(), we have
      
        if (lzeros) {
          mpi_limb_t *limb1 = (void *)p - sizeof(alimb);
          mpi_limb_t *limb2 = (void *)p - sizeof(alimb)
                                     + lzeros;
          *limb1 = *limb2;
          ...
        }
      
      where p points past the end of alimb2 which lives on the stack and contains
      the current limb in BE order.
      
      The purpose of the above is to shift the non-zero bytes of alimb2 to its
      beginning in memory, i.e. to skip its leading zero bytes.
      
      However, limb2 points somewhere into the middle of alimb2 and thus, reading
      *limb2 pulls in lzero bytes from somewhere.
      
      Indeed, KASAN splats:
      
        BUG: KASAN: stack-out-of-bounds in mpi_write_to_sgl+0x4e3/0x6f0
                                            at addr ffff8800cb04f601
        Read of size 8 by task systemd-udevd/391
        page:ffffea00032c13c0 count:0 mapcount:0 mapping:   (null) index:0x0
        flags: 0x3fff8000000000()
        page dumped because: kasan: bad access detected
        CPU: 3 PID: 391 Comm: systemd-udevd Tainted: G  B  L
                                                    4.5.0-next-20160316+ #12
        [...]
        Call Trace:
         [<ffffffff8194889e>] dump_stack+0xdc/0x15e
         [<ffffffff819487c2>] ? _atomic_dec_and_lock+0xa2/0xa2
         [<ffffffff814892b5>] ? __dump_page+0x185/0x330
         [<ffffffff8150ffd6>] kasan_report_error+0x5e6/0x8b0
         [<ffffffff814724cd>] ? kzfree+0x2d/0x40
         [<ffffffff819c5bce>] ? mpi_free_limb_space+0xe/0x20
         [<ffffffff819c469e>] ? mpi_powm+0x37e/0x16f0
         [<ffffffff815109f1>] kasan_report+0x71/0xa0
         [<ffffffff819c0353>] ? mpi_write_to_sgl+0x4e3/0x6f0
         [<ffffffff8150ed34>] __asan_load8+0x64/0x70
         [<ffffffff819c0353>] mpi_write_to_sgl+0x4e3/0x6f0
         [<ffffffff819bfe70>] ? mpi_set_buffer+0x620/0x620
         [<ffffffff819c0e6f>] ? mpi_cmp+0xbf/0x180
         [<ffffffff8186e282>] rsa_verify+0x202/0x260
      
      What's more, since lzeros can be anything from 1 to sizeof(mpi_limb_t)-1,
      the above will cause unaligned accesses which is bad on non-x86 archs.
      
      Fix the issue, by preparing the starting point p for the upcoming copy
      operation instead of shifting the source memory, i.e. alimb2.
      
      Fixes: 2d4d1eea ("lib/mpi: Add mpi sgl helpers")
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      cece762f
    • Nicolai Stange's avatar
      lib/mpi: mpi_write_sgl(): purge redundant pointer arithmetic · ea122be0
      Nicolai Stange authored
      Within the copying loop in mpi_write_sgl(), we have
      
        if (lzeros) {
          ...
          p -= lzeros;
          y = lzeros;
        }
        p = p - (sizeof(alimb) - y);
      
      If lzeros == 0, then y == 0, too. Thus, lzeros gets subtracted and added
      back again to p.
      
      Purge this redundancy.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      ea122be0
    • Nicolai Stange's avatar
      lib/mpi: mpi_write_sgl(): fix style issue with lzero decrement · 654842ef
      Nicolai Stange authored
      Within the copying loop in mpi_write_sgl(), we have
      
        if (lzeros > 0) {
          ...
          lzeros -= sizeof(alimb);
        }
      
      However, at this point, lzeros < sizeof(alimb) holds. Make this fact
      explicit by rewriting the above to
      
        if (lzeros) {
          ...
          lzeros = 0;
        }
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      654842ef
    • Nicolai Stange's avatar
      lib/mpi: mpi_write_sgl(): fix skipping of leading zero limbs · f2d1362f
      Nicolai Stange authored
      Currently, if the number of leading zeros is greater than fits into a
      complete limb, mpi_write_sgl() skips them by iterating over them limb-wise.
      
      However, it fails to adjust its internal leading zeros tracking variable,
      lzeros, accordingly: it does a
      
        p -= sizeof(alimb);
        continue;
      
      which should really have been a
      
        lzeros -= sizeof(alimb);
        continue;
      
      Since lzeros never decreases if its initial value >= sizeof(alimb), nothing
      gets copied by mpi_write_sgl() in that case.
      
      Instead of skipping the high order zero limbs within the loop as shown
      above, fix the issue by adjusting the copying loop's bounds.
      
      Fixes: 2d4d1eea ("lib/mpi: Add mpi sgl helpers")
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      f2d1362f
  2. 27 Feb, 2016 3 commits
    • Arnd Bergmann's avatar
      lib/mpi: use "static inline" instead of "extern inline" · 9c6bd0c2
      Arnd Bergmann authored
      When we use CONFIG_PROFILE_ALL_BRANCHES, every 'if()' introduces
      a static variable, but that is not allowed in 'extern inline'
      functions:
      
      mpi-inline.h:116:204: warning: '______f' is static but declared in inline function 'mpihelp_sub' which is not static
      mpi-inline.h:113:184: warning: '______f' is static but declared in inline function 'mpihelp_sub' which is not static
      mpi-inline.h:70:184: warning: '______f' is static but declared in inline function 'mpihelp_add' which is not static
      mpi-inline.h:56:204: warning: '______f' is static but declared in inline function 'mpihelp_add_1' which is not static
      
      This changes the MPI code to use 'static inline' instead, to get
      rid of hundreds of warnings.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      9c6bd0c2
    • Arnd Bergmann's avatar
      lib/mpi: avoid assembler warning · c5d55248
      Arnd Bergmann authored
      A wrapper around the umull assembly instruction might reuse
      the input register as an output, which is undefined on
      some ARM machines, as pointed out by this assembler warning:
      
        CC      lib/mpi/generic_mpih-mul1.o
      /tmp/ccxJuxIy.s: Assembler messages:
      /tmp/ccxJuxIy.s:53: rdhi, rdlo and rm must all be different
        CC      lib/mpi/generic_mpih-mul2.o
      /tmp/ccI0scAD.s: Assembler messages:
      /tmp/ccI0scAD.s:53: rdhi, rdlo and rm must all be different
        CC      lib/mpi/generic_mpih-mul3.o
      /tmp/ccMvVQcp.s: Assembler messages:
      /tmp/ccMvVQcp.s:53: rdhi, rdlo and rm must all be different
      
      This changes the constraints to force different registers to
      be used as output.
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      c5d55248
    • Michal Marek's avatar
      lib/mpi: Endianness fix · 3ee0cb5f
      Michal Marek authored
      The limbs are integers in the host endianness, so we can't simply
      iterate over the individual bytes. The current code happens to work on
      little-endian, because the order of the limbs in the MPI array is the
      same as the order of the bytes in each limb, but it breaks on
      big-endian.
      
      Fixes: 0f74fbf7 ("MPI: Fix mpi_read_buffer")
      Signed-off-by: default avatarMichal Marek <mmarek@suse.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      3ee0cb5f
  3. 17 Nov, 2015 1 commit
  4. 20 Oct, 2015 1 commit
  5. 14 Oct, 2015 2 commits
  6. 25 Aug, 2015 1 commit
  7. 16 Jun, 2015 1 commit
  8. 13 Jun, 2015 1 commit
  9. 14 Jan, 2015 3 commits
  10. 25 Sep, 2013 1 commit
  11. 19 Jul, 2013 1 commit
  12. 12 Jun, 2013 1 commit
  13. 24 May, 2013 1 commit
    • Helge Deller's avatar
      MPILIB: disable usage of floating point registers on parisc · 70ef5578
      Helge Deller authored
      The umul_ppmm() macro for parisc uses the xmpyu assembler statement
      which does calculation via a floating point register.
      
      But usage of floating point registers inside the Linux kernel are not
      allowed and gcc will stop compilation due to the -mdisable-fpregs
      compiler option.
      
      Fix this by disabling the umul_ppmm() and udiv_qrnnd() macros. The
      mpilib will then use the generic built-in implementations instead.
      Signed-off-by: default avatarHelge Deller <deller@gmx.de>
      70ef5578
  14. 01 Feb, 2013 1 commit
  15. 23 Nov, 2012 1 commit
  16. 08 Oct, 2012 3 commits
  17. 26 May, 2012 2 commits
  18. 18 Apr, 2012 1 commit
    • Jesper Juhl's avatar
      mpi: Avoid using freed pointer in mpi_lshift_limbs() · 09c79b60
      Jesper Juhl authored
      At the start of the function we assign 'a->d' to 'ap'. Then we use the
      RESIZE_IF_NEEDED macro on 'a' - this may free 'a->d' and replace it
      with newly allocaetd storage. In that case, we'll be operating on
      freed memory further down in the function when we index into 'ap[]'.
      Since we don't actually need 'ap' until after the use of the
      RESIZE_IF_NEEDED macro we can just delay the assignment to it until
      after we've potentially resized, thus avoiding the issue.
      
      While I was there anyway I also changed the integer variable 'n' to be
      const. It might as well be since we only assign to it once and use it
      as a constant, and then the compiler will tell us if we ever assign to
      it in the future.
      Signed-off-by: default avatarJesper Juhl <jj@chaosbits.net>
      Acked-by: default avatarDmitry Kasatkin <dmitry.kasatkin@intel.com>
      Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
      09c79b60
  19. 01 Feb, 2012 3 commits