1. 10 Jul, 2019 1 commit
  2. 03 Mar, 2018 1 commit
    • James Hogan's avatar
      lib/mpi: Fix umul_ppmm() for MIPS64r6 · 22d5e20c
      James Hogan authored
      
      [ Upstream commit bbc25bee ]
      
      Current MIPS64r6 toolchains aren't able to generate efficient
      DMULU/DMUHU based code for the C implementation of umul_ppmm(), which
      performs an unsigned 64 x 64 bit multiply and returns the upper and
      lower 64-bit halves of the 128-bit result. Instead it widens the 64-bit
      inputs to 128-bits and emits a __multi3 intrinsic call to perform a 128
      x 128 multiply. This is both inefficient, and it results in a link error
      since we don't include __multi3 in MIPS linux.
      
      For example commit 90a53e44 ("cfg80211: implement regdb signature
      checking") merged in v4.15-rc1 recently broke the 64r6_defconfig and
      64r6el_defconfig builds by indirectly selecting MPILIB. The same build
      errors can be reproduced on older kernels by enabling e.g. CRYPTO_RSA:
      
      lib/mpi/generic_mpih-mul1.o: In function `mpihelp_mul_1':
      lib/mpi/generic_mpih-mul1.c:50: undefined reference to `__multi3'
      lib/mpi/generic_mpih-mul2.o: In function `mpihelp_addmul_1':
      lib/mpi/generic_mpih-mul2.c:49: undefined reference to `__multi3'
      lib/mpi/generic_mpih-mul3.o: In function `mpihelp_submul_1':
      lib/mpi/generic_mpih-mul3.c:49: undefined reference to `__multi3'
      lib/mpi/mpih-div.o In function `mpihelp_divrem':
      lib/mpi/mpih-div.c:205: undefined reference to `__multi3'
      lib/mpi/mpih-div.c:142: undefined reference to `__multi3'
      
      Therefore add an efficient MIPS64r6 implementation of umul_ppmm() using
      inline assembly and the DMULU/DMUHU instructions, to prevent __multi3
      calls being emitted.
      
      Fixes: 7fd08ca5 ("MIPS: Add build support for the MIPS R6 ISA")
      Signed-off-by: default avatarJames Hogan <jhogan@kernel.org>
      Cc: Ralf Baechle <ralf@linux-mips.org>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: "David S. Miller" <davem@davemloft.net>
      Cc: linux-mips@linux-mips.org
      Cc: linux-crypto@vger.kernel.org
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarSasha Levin <alexander.levin@verizon.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      22d5e20c
  3. 30 Nov, 2017 1 commit
    • Eric Biggers's avatar
      lib/mpi: call cond_resched() from mpi_powm() loop · ce922b7b
      Eric Biggers authored
      commit 1d9ddde1 upstream.
      
      On a non-preemptible kernel, if KEYCTL_DH_COMPUTE is called with the
      largest permitted inputs (16384 bits), the kernel spends 10+ seconds
      doing modular exponentiation in mpi_powm() without rescheduling.  If all
      threads do it, it locks up the system.  Moreover, it can cause
      rcu_sched-stall warnings.
      
      Notwithstanding the insanity of doing this calculation in kernel mode
      rather than in userspace, fix it by calling cond_resched() as each bit
      from the exponent is processed.  It's still noninterruptible, but at
      least it's preemptible now.
      
      Do the cond_resched() once per bit rather than once per MPI limb because
      each limb might still easily take 100+ milliseconds on slow CPUs.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      ce922b7b
  4. 02 Nov, 2017 1 commit
    • Greg Kroah-Hartman's avatar
      License cleanup: add SPDX GPL-2.0 license identifier to files with no license · b2441318
      Greg Kroah-Hartman authored
      Many source files in the tree are missing licensing information, which
      makes it harder for compliance tools to determine the correct license.
      
      By default all files without license information are under the default
      license of the kernel, which is GPL version 2.
      
      Update the files which contain no license information with the 'GPL-2.0'
      SPDX license identifier.  The SPDX identifier is a legally binding
      shorthand, which can be used instead of the full boiler plate text.
      
      This patch is based on work done by Thomas Gleixner and Kate Stewart and
      Philippe Ombredanne.
      
      How this work was done:
      
      Patches were generated and checked against linux-4.14-rc6 for a subset of
      the use cases:
       - file had no licensing information it it.
       - file was a */uapi/* one with no licensing information in it,
       - file was a */uapi/* one with existing licensing information,
      
      Further patches will be generated in subsequent months to fix up cases
      where non-standard license headers were used, and references to license
      had to be inferred by heuristics based on keywords.
      
      The analysis to determine which SPDX License Identifier to be applied to
      a file was done in a spreadsheet of side by side results from of the
      output of two independent scanners (ScanCode & Windriver) producing SPDX
      tag:value files created by Philippe Ombredanne.  Philippe prepared the
      base worksheet, and did an initial spot review of a few 1000 files.
      
      The 4.13 kernel was the starting point of the analysis with 60,537 files
      assessed.  Kate Stewart did a file by file comparison of the scanner
      results in the spreadsheet to determine which SPDX license identifier(s)
      to be applied to the file. She confirmed any determination that was not
      immediately clear with lawyers working with the Linux Foundation.
      
      Criteria used to select files for SPDX license identifier tagging was:
       - Files considered eligible had to be source code files.
       - Make and config files were included as candidates if they contained >5
         lines of source
       - File already had some variant of a license header in it (even if <5
         lines).
      
      All documentation files were explicitly excluded.
      
      The following heuristics were used to determine which SPDX license
      identifiers to apply.
      
       - when both scanners couldn't find any license traces, file was
         considered to have no license information in it, and the top level
         COPYING file license applied.
      
         For non */uapi/* files that summary was:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|-------
         GPL-2.0                                              11139
      
         and resulted in the first patch in this series.
      
         If that file was a */uapi/* path one, it was "GPL-2.0 WITH
         Linux-syscall-note" otherwise it was "GPL-2.0".  Results of that was:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|-------
         GPL-2.0 WITH Linux-syscall-note                        930
      
         and resulted in the second patch in this series.
      
       - if a file had some form of licensing information in it, and was one
         of the */uapi/* ones, it was denoted with the Linux-syscall-note if
         any GPL family license was found in the file or had no licensing in
         it (per prior point).  Results summary:
      
         SPDX license identifier                            # files
         ---------------------------------------------------|------
         GPL-2.0 WITH Linux-syscall-note                       270
         GPL-2.0+ WITH Linux-syscall-note                      169
         ((GPL-2.0 WITH Linux-syscall-note) OR BSD-2-Clause)    21
         ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)    17
         LGPL-2.1+ WITH Linux-syscall-note                      15
         GPL-1.0+ WITH Linux-syscall-note                       14
         ((GPL-2.0+ WITH Linux-syscall-note) OR BSD-3-Clause)    5
         LGPL-2.0+ WITH Linux-syscall-note                       4
         LGPL-2.1 WITH Linux-syscall-note                        3
         ((GPL-2.0 WITH Linux-syscall-note) OR MIT)              3
         ((GPL-2.0 WITH Linux-syscall-note) AND MIT)             1
      
         and that resulted in the third patch in this series.
      
       - when the two scanners agreed on the detected license(s), that became
         the concluded license(s).
      
       - when there was disagreement between the two scanners (one detected a
         license but the other didn't, or they both detected different
         licenses) a manual inspection of the file occurred.
      
       - In most cases a manual inspection of the information in the file
         resulted in a clear resolution of the license that should apply (and
         which scanner probably needed to revisit its heuristics).
      
       - When it was not immediately clear, the license identifier was
         confirmed with lawyers working with the Linux Foundation.
      
       - If there was any question as to the appropriate license identifier,
         the file was flagged for further research and to be revisited later
         in time.
      
      In total, over 70 hours of logged manual review was done on the
      spreadsheet to determine the SPDX license identifiers to apply to the
      source files by Kate, Philippe, Thomas and, in some cases, confirmation
      by lawyers working with the Linux Foundation.
      
      Kate also obtained a third independent scan of the 4.13 code base from
      FOSSology, and compared selected files where the other two scanners
      disagreed against that SPDX file, to see if there was new insights.  The
      Windriver scanner is based on an older version of FOSSology in part, so
      they are related.
      
      Thomas did random spot checks in about 500 files from the spreadsheets
      for the uapi headers and agreed with SPDX license identifier in the
      files he inspected. For the non-uapi files Thomas did random spot checks
      in about 15000 files.
      
      In initial set of patches against 4.14-rc6, 3 files were found to have
      copy/paste license identifier errors, and have been fixed to reflect the
      correct identifier.
      
      Additionally Philippe spent 10 hours this week doing a detailed manual
      inspection and review of the 12,461 patched files from the initial patch
      version early this week with:
       - a full scancode scan run, collecting the matched texts, detected
         license ids and scores
       - reviewing anything where there was a license detected (about 500+
         files) to ensure that the applied SPDX license was correct
       - reviewing anything where there was no detection but the patch license
         was not GPL-2.0 WITH Linux-syscall-note to ensure that the applied
         SPDX license was correct
      
      This produced a worksheet with 20 files needing minor correction.  This
      worksheet was then exported into 3 different .csv files for the
      different types of files to be modified.
      
      These .csv files were then reviewed by Greg.  Thomas wrote a script to
      parse the csv files and add the proper SPDX tag to the file, in the
      format that the file expected.  This script was further refined by Greg
      based on the output to detect more types of files automatically and to
      distinguish between header and source .c files (which need different
      comment types.)  Finally Greg ran the script using the .csv files to
      generate the patches.
      Reviewed-by: default avatarKate Stewart <kstewart@linuxfoundation.org>
      Reviewed-by: default avatarPhilippe Ombredanne <pombredanne@nexb.com>
      Reviewed-by: default avatarThomas Gleixner <tglx@linutronix.de>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      b2441318
  5. 22 Aug, 2017 1 commit
  6. 17 Aug, 2017 1 commit
    • Stefan Agner's avatar
      lib/mpi: fix build with clang · dea632ca
      Stefan Agner authored
      Use just @ to denote comments which works with gcc and clang.
      Otherwise clang reports an escape sequence error:
        error: invalid % escape in inline assembly string
      
      Use %0-%3 as operand references, this avoids:
        error: invalid operand in inline asm: 'umull ${1:r}, ${0:r}, ${2:r}, ${3:r}'
      
      Also remove superfluous casts on output operands to avoid warnings
      such as:
        warning: invalid use of a cast in an inline asm context requiring an l-value
      Signed-off-by: default avatarStefan Agner <stefan@agner.ch>
      Acked-by: default avatarArnd Bergmann <arnd@arndb.de>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      dea632ca
  7. 25 Nov, 2016 1 commit
    • Andrey Ryabinin's avatar
      mpi: Fix NULL ptr dereference in mpi_powm() [ver #3] · f5527fff
      Andrey Ryabinin authored
      This fixes CVE-2016-8650.
      
      If mpi_powm() is given a zero exponent, it wants to immediately return
      either 1 or 0, depending on the modulus.  However, if the result was
      initalised with zero limb space, no limbs space is allocated and a
      NULL-pointer exception ensues.
      
      Fix this by allocating a minimal amount of limb space for the result when
      the 0-exponent case when the result is 1 and not touching the limb space
      when the result is 0.
      
      This affects the use of RSA keys and X.509 certificates that carry them.
      
      BUG: unable to handle kernel NULL pointer dereference at           (null)
      IP: [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6
      PGD 0
      Oops: 0002 [#1] SMP
      Modules linked in:
      CPU: 3 PID: 3014 Comm: keyctl Not tainted 4.9.0-rc6-fscache+ #278
      Hardware name: ASUS All Series/H97-PLUS, BIOS 2306 10/09/2014
      task: ffff8804011944c0 task.stack: ffff880401294000
      RIP: 0010:[<ffffffff8138ce5d>]  [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6
      RSP: 0018:ffff880401297ad8  EFLAGS: 00010212
      RAX: 0000000000000000 RBX: ffff88040868bec0 RCX: ffff88040868bba0
      RDX: ffff88040868b260 RSI: ffff88040868bec0 RDI: ffff88040868bee0
      RBP: ffff880401297ba8 R08: 0000000000000000 R09: 0000000000000000
      R10: 0000000000000047 R11: ffffffff8183b210 R12: 0000000000000000
      R13: ffff8804087c7600 R14: 000000000000001f R15: ffff880401297c50
      FS:  00007f7a7918c700(0000) GS:ffff88041fb80000(0000) knlGS:0000000000000000
      CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      CR2: 0000000000000000 CR3: 0000000401250000 CR4: 00000000001406e0
      Stack:
       ffff88040868bec0 0000000000000020 ffff880401297b00 ffffffff81376cd4
       0000000000000100 ffff880401297b10 ffffffff81376d12 ffff880401297b30
       ffffffff81376f37 0000000000000100 0000000000000000 ffff880401297ba8
      Call Trace:
       [<ffffffff81376cd4>] ? __sg_page_iter_next+0x43/0x66
       [<ffffffff81376d12>] ? sg_miter_get_next_page+0x1b/0x5d
       [<ffffffff81376f37>] ? sg_miter_next+0x17/0xbd
       [<ffffffff8138ba3a>] ? mpi_read_raw_from_sgl+0xf2/0x146
       [<ffffffff8132a95c>] rsa_verify+0x9d/0xee
       [<ffffffff8132acca>] ? pkcs1pad_sg_set_buf+0x2e/0xbb
       [<ffffffff8132af40>] pkcs1pad_verify+0xc0/0xe1
       [<ffffffff8133cb5e>] public_key_verify_signature+0x1b0/0x228
       [<ffffffff8133d974>] x509_check_for_self_signed+0xa1/0xc4
       [<ffffffff8133cdde>] x509_cert_parse+0x167/0x1a1
       [<ffffffff8133d609>] x509_key_preparse+0x21/0x1a1
       [<ffffffff8133c3d7>] asymmetric_key_preparse+0x34/0x61
       [<ffffffff812fc9f3>] key_create_or_update+0x145/0x399
       [<ffffffff812fe227>] SyS_add_key+0x154/0x19e
       [<ffffffff81001c2b>] do_syscall_64+0x80/0x191
       [<ffffffff816825e4>] entry_SYSCALL64_slow_path+0x25/0x25
      Code: 56 41 55 41 54 53 48 81 ec a8 00 00 00 44 8b 71 04 8b 42 04 4c 8b 67 18 45 85 f6 89 45 80 0f 84 b4 06 00 00 85 c0 75 2f 41 ff ce <49> c7 04 24 01 00 00 00 b0 01 75 0b 48 8b 41 18 48 83 38 01 0f
      RIP  [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6
       RSP <ffff880401297ad8>
      CR2: 0000000000000000
      ---[ end trace d82015255d4a5d8d ]---
      
      Basically, this is a backport of a libgcrypt patch:
      
      	http://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=patch;h=6e1adb05d290aeeb1c230c763970695f4a538526
      
      Fixes: cdec9cb5 ("crypto: GnuPG based MPI lib - source files (part 1)")
      Signed-off-by: default avatarAndrey Ryabinin <aryabinin@virtuozzo.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
      cc: linux-ima-devel@lists.sourceforge.net
      cc: stable@vger.kernel.org
      Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
      f5527fff
  8. 29 Jul, 2016 1 commit
  9. 01 Jul, 2016 2 commits
    • Herbert Xu's avatar
      lib/mpi: Do not do sg_virt · 127827b9
      Herbert Xu authored
      Currently the mpi SG helpers use sg_virt which is completely
      broken.  It happens to work with normal kernel memory but will
      fail with anything that is not linearly mapped.
      
      This patch fixes this by using the SG iterator helpers.
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      127827b9
    • Herbert Xu's avatar
      crypto: rsa - Generate fixed-length output · 9b45b7bb
      Herbert Xu authored
      Every implementation of RSA that we have naturally generates
      output with leading zeroes.  The one and only user of RSA,
      pkcs1pad wants to have those leading zeroes in place, in fact
      because they are currently absent it has to write those zeroes
      itself.
      
      So we shouldn't be stripping leading zeroes in the first place.
      In fact this patch makes rsa-generic produce output with fixed
      length so that pkcs1pad does not need to do any extra work.
      
      This patch also changes DH to use the new interface.
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      9b45b7bb
  10. 31 May, 2016 7 commits
    • Nicolai Stange's avatar
      lib/mpi: refactor mpi_read_from_buffer() in terms of mpi_read_raw_data() · 20b5b7f3
      Nicolai Stange authored
      mpi_read_from_buffer() and mpi_read_raw_data() do basically the same thing
      except that the former extracts the number of payload bits from the first
      two bytes of the input buffer.
      
      Besides that, the data copying logic is exactly the same.
      
      Replace the open coded buffer to MPI instance conversion by a call to
      mpi_read_raw_data().
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      20b5b7f3
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk · cdf24b42
      Nicolai Stange authored
      The first two bytes of the input buffer encode its expected length and
      mpi_read_from_buffer() prints a console message if the given buffer is too
      short.
      
      However, there are some oddities with how this message is printed:
      - It is printed at the default loglevel. This is different from the
        one used in the case that the first two bytes' value is unsupportedly
        large, i.e. KERN_INFO.
      - The format specifier '%d' is used for unsigned ints.
      - It prints the values of nread and *ret_nread. This is redundant since
        the former is always the latter + 1.
      
      Clean this up as follows:
      - Use pr_info() rather than printk() with no loglevel.
      - Use the format specifiers '%u' in place if '%d'.
      - Do not print the redundant 'nread' but the more helpful 'nbytes' value.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      cdf24b42
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_from_buffer(): return -EINVAL upon too short buffer · 7af791e0
      Nicolai Stange authored
      Currently, if the input buffer is shorter than the expected length as
      indicated by its first two bytes, an MPI instance of this expected length
      will be allocated and filled with as much data as is available. The rest
      will remain uninitialized.
      
      Instead of leaving this condition undetected, an error code should be
      reported to the caller.
      
      Since this situation indicates that the input buffer's first two bytes,
      encoding the number of expected bits, are garbled, -EINVAL is appropriate
      here.
      
      If the input buffer is shorter than indicated by its first two bytes,
      make mpi_read_from_buffer() return -EINVAL.
      Get rid of the 'nread' variable: with the new semantics, the total number
      of bytes read from the input buffer is known in advance.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      7af791e0
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_from_buffer(): return error code · 03cdfaad
      Nicolai Stange authored
      mpi_read_from_buffer() reads a MPI from a buffer into a newly allocated
      MPI instance. It expects the buffer's leading two bytes to contain the
      number of bits, followed by the actual payload.
      
      On failure, it returns NULL and updates the in/out argument ret_nread
      somewhat inconsistently:
      - If the given buffer is too short to contain the leading two bytes
        encoding the number of bits or their value is unsupported, then
        ret_nread will be cleared.
      - If the allocation of the resulting MPI instance fails, ret_nread is left
        as is.
      
      The only user of mpi_read_from_buffer(), digsig_verify_rsa(), simply checks
      for a return value of NULL and returns -ENOMEM if that happens.
      
      While this is all of cosmetic nature only, there is another error condition
      which currently isn't detectable by the caller of mpi_read_from_buffer():
      if the given buffer is too small to hold the number of bits as encoded in
      its first two bytes, the return value will be non-NULL and *ret_nread > 0.
      
      In preparation of communicating this condition to the caller, let
      mpi_read_from_buffer() return error values by means of the ERR_PTR()
      mechanism.
      
      Make the sole caller of mpi_read_from_buffer(), digsig_verify_rsa(),
      check the return value for IS_ERR() rather than == NULL. If IS_ERR() is
      true, return the associated error value rather than the fixed -ENOMEM.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      03cdfaad
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_raw_data(): fix nbits calculation · eef0df6a
      Nicolai Stange authored
      The number of bits, nbits, is calculated in mpi_read_raw_data() as follows:
      
        nbits = nbytes * 8;
      
      Afterwards, the number of leading zero bits of the first byte get
      subtracted:
      
        nbits -= count_leading_zeros(buffer[0]);
      
      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: e1045992 ("MPILIB: Provide a function to read raw data into an
                           MPI")
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      eef0df6a
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_raw_data(): purge redundant clearing of nbits · dfd90510
      Nicolai Stange authored
      In mpi_read_raw_data(), 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>
      dfd90510
    • Nicolai Stange's avatar
      lib/mpi: purge mpi_set_buffer() · 4bdf1cfc
      Nicolai Stange authored
      mpi_set_buffer() has no in-tree users and similar functionality is provided
      by mpi_read_raw_data().
      
      Remove mpi_set_buffer().
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      4bdf1cfc
  11. 05 Apr, 2016 14 commits
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_raw_from_sgl(): fix out-of-bounds buffer access · 0bb5c9ea
      Nicolai Stange authored
      Within the copying loop in mpi_read_raw_from_sgl(), the last input SGE's
      byte count gets artificially extended as follows:
      
        if (sg_is_last(sg) && (len % BYTES_PER_MPI_LIMB))
          len += BYTES_PER_MPI_LIMB - (len % BYTES_PER_MPI_LIMB);
      
      Within the following byte copying loop, this causes reads beyond that
      SGE's allocated buffer:
      
        BUG: KASAN: slab-out-of-bounds in mpi_read_raw_from_sgl+0x331/0x650
                                           at addr ffff8801e168d4d8
        Read of size 1 by task systemd-udevd/721
        [...]
        Call Trace:
         [<ffffffff818c4d35>] dump_stack+0xbc/0x117
         [<ffffffff818c4c79>] ? _atomic_dec_and_lock+0x169/0x169
         [<ffffffff814af5d1>] ? print_section+0x61/0xb0
         [<ffffffff814b1109>] print_trailer+0x179/0x2c0
         [<ffffffff814bc524>] object_err+0x34/0x40
         [<ffffffff814bfdc7>] kasan_report_error+0x307/0x8c0
         [<ffffffff814bf315>] ? kasan_unpoison_shadow+0x35/0x50
         [<ffffffff814bf38e>] ? kasan_kmalloc+0x5e/0x70
         [<ffffffff814c0ad1>] kasan_report+0x71/0xa0
         [<ffffffff81938171>] ? mpi_read_raw_from_sgl+0x331/0x650
         [<ffffffff814bf1a6>] __asan_load1+0x46/0x50
         [<ffffffff81938171>] mpi_read_raw_from_sgl+0x331/0x650
         [<ffffffff817f41b6>] rsa_verify+0x106/0x260
         [<ffffffff817f40b0>] ? rsa_set_pub_key+0xf0/0xf0
         [<ffffffff818edc79>] ? sg_init_table+0x29/0x50
         [<ffffffff817f4d22>] ? pkcs1pad_sg_set_buf+0xb2/0x2e0
         [<ffffffff817f5b74>] pkcs1pad_verify+0x1f4/0x2b0
         [<ffffffff81831057>] public_key_verify_signature+0x3a7/0x5e0
         [<ffffffff81830cb0>] ? public_key_describe+0x80/0x80
         [<ffffffff817830f0>] ? keyring_search_aux+0x150/0x150
         [<ffffffff818334a4>] ? x509_request_asymmetric_key+0x114/0x370
         [<ffffffff814b83f0>] ? kfree+0x220/0x370
         [<ffffffff818312c2>] public_key_verify_signature_2+0x32/0x50
         [<ffffffff81830b5c>] verify_signature+0x7c/0xb0
         [<ffffffff81835d0c>] pkcs7_validate_trust+0x42c/0x5f0
         [<ffffffff813c391a>] system_verify_data+0xca/0x170
         [<ffffffff813c3850>] ? top_trace_array+0x9b/0x9b
         [<ffffffff81510b29>] ? __vfs_read+0x279/0x3d0
         [<ffffffff8129372f>] mod_verify_sig+0x1ff/0x290
        [...]
      
      The exact purpose of the len extension isn't clear to me, but due to
      its form, I suspect that it's a leftover somehow accounting for leading
      zero bytes within the most significant output limb.
      
      Note however that without that len adjustement, the total number of bytes
      ever processed by the inner loop equals nbytes and thus, the last output
      limb gets written at this point. Thus the net effect of the len adjustement
      cited above is just to keep the inner loop running for some more
      iterations, namely < BYTES_PER_MPI_LIMB ones, reading some extra bytes from
      beyond the last SGE's buffer and discarding them afterwards.
      
      Fix this issue by purging the extension of len beyond the last input SGE's
      buffer length.
      
      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>
      0bb5c9ea
    • Nicolai Stange's avatar
      lib/mpi: mpi_read_raw_from_sgl(): sanitize meaning of indices · 85d541a3
      Nicolai Stange authored
      Within the byte reading loop in mpi_read_raw_sgl(), there are two
      housekeeping indices used, z and x.
      
      At all times, the index z represents the number of output bytes covered
      by the input SGEs for which processing has completed so far. This includes
      any leading zero bytes within the most significant limb.
      
      The index x changes its meaning after the first outer loop's iteration
      though: while processing the first input SGE, it represents
      
        "number of leading zero bytes in most significant output limb" +
         "current position within current SGE"
      
      For the remaining SGEs OTOH, x corresponds just to
      
        "current position within current SGE"
      
      After all, it is only the sum of z and x that has any meaning for the
      output buffer and thus, the
      
        "number of leading zero bytes in most significant output limb"
      
      part can be moved away from x into z from the beginning, opening up the
      opportunity for cleaner code.
      
      Before the outer loop iterating over the SGEs, don't initialize z with
      zero, but with the number of leading zero bytes in the most significant
      output limb. For the inner loop iterating over a single SGE's bytes,
      get rid of the buf_shift offset to x' bounds and let x run from zero to
      sg->length - 1.
      Signed-off-by: default avatarNicolai Stange <nicstange@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      85d541a3
    • 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
  12. 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
  13. 17 Nov, 2015 1 commit
  14. 20 Oct, 2015 1 commit
  15. 14 Oct, 2015 2 commits
  16. 25 Aug, 2015 1 commit
  17. 16 Jun, 2015 1 commit