On Thu, Jun 20, 2013 at 10:00:21AM +0200, Daniel Borkmann wrote:
> After having fixed a NULL pointer dereference in SCTP 1abd165e ("net:
> sctp: fix NULL pointer dereference in socket destruction"), I ran into
> the following NULL pointer dereference in the crypto subsystem with
> the same reproducer, easily hit each time:
> 
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffff81070321>] __wake_up_common+0x31/0x90
> PGD 0
> Oops: 0000 [#1] SMP
> Modules linked in: padlock_sha(F-) sha256_generic(F) sctp(F) libcrc32c(F) [..]
> CPU: 6 PID: 3326 Comm: cryptomgr_probe Tainted: GF            3.10.0-rc5+ #1
> Hardware name: Dell Inc. PowerEdge T410/0H19HD, BIOS 1.6.3 02/01/2011
> task: ffff88007b6cf4e0 ti: ffff88007b7cc000 task.ti: ffff88007b7cc000
> RIP: 0010:[<ffffffff81070321>]  [<ffffffff81070321>] __wake_up_common+0x31/0x90
> RSP: 0018:ffff88007b7cde08  EFLAGS: 00010082
> RAX: ffffffffffffffe8 RBX: ffff88003756c130 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000003 RDI: ffff88003756c130
> RBP: ffff88007b7cde48 R08: 0000000000000000 R09: ffff88012b173200
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000282
> R13: ffff88003756c138 R14: 0000000000000000 R15: 0000000000000000
> FS:  0000000000000000(0000) GS:ffff88012fc60000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000000 CR3: 0000000001a0b000 CR4: 00000000000007e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Stack:
>  ffff88007b7cde28 0000000300000000 ffff88007b7cde28 ffff88003756c130
>  0000000000000282 ffff88003756c128 ffffffff81227670 0000000000000000
>  ffff88007b7cde78 ffffffff810722b7 ffff88007cdcf000 ffffffff81a90540
> Call Trace:
>  [<ffffffff81227670>] ? crypto_alloc_pcomp+0x20/0x20
>  [<ffffffff810722b7>] complete_all+0x47/0x60
>  [<ffffffff81227708>] cryptomgr_probe+0x98/0xc0
>  [<ffffffff81227670>] ? crypto_alloc_pcomp+0x20/0x20
>  [<ffffffff8106760e>] kthread+0xce/0xe0
>  [<ffffffff81067540>] ? kthread_freezable_should_stop+0x70/0x70
>  [<ffffffff815450dc>] ret_from_fork+0x7c/0xb0
>  [<ffffffff81067540>] ? kthread_freezable_should_stop+0x70/0x70
> Code: 41 56 41 55 41 54 53 48 83 ec 18 66 66 66 66 90 89 75 cc 89 55 c8
>       4c 8d 6f 08 48 8b 57 08 41 89 cf 4d 89 c6 48 8d 42 e
> RIP  [<ffffffff81070321>] __wake_up_common+0x31/0x90
>  RSP <ffff88007b7cde08>
> CR2: 0000000000000000
> ---[ end trace b495b19270a4d37e ]---
> 
> My assumption is that the following is happening: the minimal SCTP
> tool runs under ``echo 1 > /proc/sys/net/sctp/auth_enable'', hence
> it's making use of crypto_alloc_hash() via sctp_auth_init_hmacs().
> It forks itself, heavily allocates, binds, listens and waits in
> accept on sctp sockets, and then randomly kills some of them (no
> need for an actual client in this case to hit this). Then, again,
> allocating, binding, etc, and then killing child processes.
> 
> The problem that might be happening here is that cryptomgr requests
> the module to probe/load through cryptomgr_schedule_probe(), but
> before the thread handler cryptomgr_probe() returns, we return from
> the wait_for_completion_interruptible() function and probably already
> have cleared up larval, thus we run into a NULL pointer dereference
> when in cryptomgr_probe() complete_all() is being called.
> 
> If we wait with wait_for_completion() instead, this panic will not
> occur anymore. This is valid, because in case a signal is pending,
> cryptomgr_probe() returns from probing anyway with properly calling
> complete_all().

The use of wait_for_completion_interruptible is intentional so that
we don't lock up the thread if a bug causes us to never wake up.

This bug is caused by the helper thread using the larval without
holding a reference count on it.  If the helper thread completes
after the original thread requesting for help has gone away and
destroyed the larval, then we get the crash above.

So the fix is to hold a reference count on the larval.

Cc: <stable@vger.kernel.org> # 3.6+
Reported-by: NDaniel Borkmann <dborkman@redhat.com>
Tested-by: NDaniel Borkmann <dborkman@redhat.com>
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

git commit 39871037 (crypto: algapi - Move larval completion
into algboss) replaced accidentally a call to complete_all() by
a call to complete(). This causes a hang on crypto allocation
if we have more than one larval waiter. This pach restores the
call to complete_all().
Signed-off-by: NSteffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

It has been observed that sometimes the crypto allocation code
will get stuck for 60 seconds or multiples thereof.  This is
usually caused by an algorithm failing to pass the self-test.

If an algorithm fails to be constructed, we will immediately notify
all larval waiters.  However, if it succeeds in construction, but
then fails the self-test, we won't notify anyone at all.

This patch fixes this by merging the notification in the case
where the algorithm fails to be constructed with that of the
the case where it pases the self-test.  This way regardless of
what happens, we'll give the larval waiters an answer.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

This patch fixes a serious bug in the test disabling patch where
it can cause an spurious load of the cryptomgr module even when
it's compiled in.

It also negates the test disabling option so that its absence
causes tests to be enabled.

The Kconfig option is also now behind EMBEDDED.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

By default, CONFIG_CRYPTO_MANAGER_TESTS will be enabled and thus
self-tests will still run, but it is now possible to disable them
to gain some time during bootup.
Signed-off-by: NAlexander Shishkin <virtuoso@slind.org>
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

include cleanup: Update gfp.h and slab.h includes to prepare for breaking implicit slab.h inclusion from percpu.h

percpu.h is included by sched.h and module.h and thus ends up being
included when building most .c files.  percpu.h includes slab.h which
in turn includes gfp.h making everything defined by the two files
universally available and complicating inclusion dependencies.

percpu.h -> slab.h dependency is about to be removed.  Prepare for
this change by updating users of gfp and slab facilities include those
headers directly instead of assuming availability.  As this conversion
needs to touch large number of source files, the following script is
used as the basis of conversion.

  http://userweb.kernel.org/~tj/misc/slabh-sweep.py

The script does the followings.

* Scan files for gfp and slab usages and update includes such that
  only the necessary includes are there.  ie. if only gfp is used,
  gfp.h, if slab is used, slab.h.

* When the script inserts a new include, it looks at the include
  blocks and try to put the new include such that its order conforms
  to its surrounding.  It's put in the include block which contains
  core kernel includes, in the same order that the rest are ordered -
  alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
  doesn't seem to be any matching order.

* If the script can't find a place to put a new include (mostly
  because the file doesn't have fitting include block), it prints out
  an error message indicating which .h file needs to be added to the
  file.

The conversion was done in the following steps.

1. The initial automatic conversion of all .c files updated slightly
   over 4000 files, deleting around 700 includes and adding ~480 gfp.h
   and ~3000 slab.h inclusions.  The script emitted errors for ~400
   files.

2. Each error was manually checked.  Some didn't need the inclusion,
   some needed manual addition while adding it to implementation .h or
   embedding .c file was more appropriate for others.  This step added
   inclusions to around 150 files.

3. The script was run again and the output was compared to the edits
   from #2 to make sure no file was left behind.

4. Several build tests were done and a couple of problems were fixed.
   e.g. lib/decompress_*.c used malloc/free() wrappers around slab
   APIs requiring slab.h to be added manually.

5. The script was run on all .h files but without automatically
   editing them as sprinkling gfp.h and slab.h inclusions around .h
   files could easily lead to inclusion dependency hell.  Most gfp.h
   inclusion directives were ignored as stuff from gfp.h was usually
   wildly available and often used in preprocessor macros.  Each
   slab.h inclusion directive was examined and added manually as
   necessary.

6. percpu.h was updated not to include slab.h.

7. Build test were done on the following configurations and failures
   were fixed.  CONFIG_GCOV_KERNEL was turned off for all tests (as my
   distributed build env didn't work with gcov compiles) and a few
   more options had to be turned off depending on archs to make things
   build (like ipr on powerpc/64 which failed due to missing writeq).

   * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
   * powerpc and powerpc64 SMP allmodconfig
   * sparc and sparc64 SMP allmodconfig
   * ia64 SMP allmodconfig
   * s390 SMP allmodconfig
   * alpha SMP allmodconfig
   * um on x86_64 SMP allmodconfig

8. percpu.h modifications were reverted so that it could be applied as
   a separate patch and serve as bisection point.

Given the fact that I had only a couple of failures from tests on step
6, I'm fairly confident about the coverage of this conversion patch.
If there is a breakage, it's likely to be something in one of the arch
headers which should be easily discoverable easily on most builds of
the specific arch.
Signed-off-by: NTejun Heo <tj@kernel.org>
Guess-its-ok-by: NChristoph Lameter <cl@linux-foundation.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

This patch introduces the template->create function intended
to replace the existing alloc function.  The intention is for
create to handle the registration directly, whereas currently
the caller of alloc has to handle the registration.

This allows type-specific code to be run prior to registration.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

We currently allocate temporary memory that is used for testing
statically.  This renders the testing engine non-reentrant. As
algorithms may nest, i.e., one may construct another in order to
carry out a part of its operation, this is unacceptable.  For
example, it has been reported that an AEAD implementation allocates
a cipher in its setkey function, which causes it to fail during
testing as the temporary memory is overwritten.

This patch replaces the static memory with dynamically allocated
buffers.  We need a maximum of 16 pages so this slightly increases
the chances of an algorithm failing due to memory shortage.
However, as testing usually occurs at registration, this shouldn't
be a big problem.
Reported-by: NShasi Pulijala <spulijala@amcc.com>
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

As it is an skcipher with no IV escapes testing altogether because
we only test givcipher objects.  This patch fixes the bypass logic
to test these algorithms.

Conversely, we're currently testing nivaead algorithms with IVs,
which would have deadlocked had it not been for the fact that no
nivaead algorithms have any test vectors.  This patch also fixes
that case.

Both fixes are ugly as hell, but this ugliness should hopefully
disappear once we move them into the per-type code (i.e., the
AEAD test would live in aead.c and the skcipher stuff in ablkcipher.c).
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

As it is we only test ciphers when combined with a mode.  That means
users that do not invoke a mode of operations may get an untested
cipher.

This patch tests all ciphers using the ECB mode so that simple cipher
users such as ansi-cprng are also protected.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

This patch makes use of the new testing infrastructure by requiring
algorithms to pass a run-time test before they're made available to
users.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

This patch moves the newly created alg_test infrastructure into
cryptomgr.  This shall allow us to use it for testing at algorithm
registrations.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

As Joy Latten points out, inner algorithm parameters will miss the closing
bracket which will also cause the outer algorithm to terminate prematurely.

This patch fixes that also kills the WARN_ON if the number of parameters
exceed the maximum as that is a user error.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

This patch adds support for having multiple parameters to
a template, separated by a comma.  It also adds support
for integer parameters in addition to the current algorithm
parameter type.

This will be used by the authenc template which will have
four parameters: the authentication algorithm, the encryption
algorithm, the authentication size and the encryption key
length.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

By the time kthread_run returns the param may have already been freed
so writing the returned thread_struct pointer to param is wrong.

In fact, we don't need it in param anyway so this patch simply puts it
on the stack.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

This patch allows the use of nested templates by allowing the use of
brackets inside a template parameter.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

This patch passes the type/mask along when constructing instances of
templates.  This is in preparation for templates that may support
multiple types of instances depending on what is requested.  For example,
the planned software async crypto driver will use this construct.

For the moment this allows us to check whether the instance constructed
is of the correct type and avoid returning success if the type does not
match.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>

Pass the work_struct pointer to the work function rather than context data.
The work function can use container_of() to work out the data.

For the cases where the container of the work_struct may go away the moment the
pending bit is cleared, it is made possible to defer the release of the
structure by deferring the clearing of the pending bit.

To make this work, an extra flag is introduced into the management side of the
work_struct.  This governs auto-release of the structure upon execution.

Ordinarily, the work queue executor would release the work_struct for further
scheduling or deallocation by clearing the pending bit prior to jumping to the
work function.  This means that, unless the driver makes some guarantee itself
that the work_struct won't go away, the work function may not access anything
else in the work_struct or its container lest they be deallocated..  This is a
problem if the auxiliary data is taken away (as done by the last patch).

However, if the pending bit is *not* cleared before jumping to the work
function, then the work function *may* access the work_struct and its container
with no problems.  But then the work function must itself release the
work_struct by calling work_release().

In most cases, automatic release is fine, so this is the default.  Special
initiators exist for the non-auto-release case (ending in _NAR).
Signed-Off-By: NDavid Howells <dhowells@redhat.com>

Spawns lock a specific crypto algorithm in place.  They can then be used
with crypto_spawn_tfm to allocate a tfm for that algorithm.  When the base
algorithm of a spawn is deregistered, all its spawns will be automatically
removed.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

This patch also adds the infrastructure to pick an algorithm based on
their type.  For example, this allows you to select the encryption
algorithm "aes", instead of any algorithm registered under the name
"aes".  For now this is only accessible internally.  Eventually it
will be made available through crypto_alloc_tfm.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>

The cryptomgr module is a simple manager of crypto algorithm instances.
It ensures that parameterised algorithms of the type tmpl(alg) (e.g.,
cbc(aes)) are always created.

This is meant to satisfy the needs for most users.  For more complex
cases such as deeper combinations or multiple parameters, a netlink
module will be created which allows arbitrary expressions to be parsed
in user-space.
Signed-off-by: NHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: NDavid S. Miller <davem@davemloft.net>