linux-stable.git/crypto/algif_skcipher.c, branch linux-4.4.y

crypto: af_alg - fix use-after-free in af_alg_accept() due to bh_lock_sock()

2020-07-09T07:35:08+00:00

commit 34c86f4c4a7be3b3e35aa48bd18299d4c756064d upstream.

The locking in af_alg_release_parent is broken as the BH socket
lock can only be taken if there is a code-path to handle the case
where the lock is owned by process-context.  Instead of adding
such handling, we can fix this by changing the ref counts to
atomic_t.

This patch also modifies the main refcnt to include both normal
and nokey sockets.  This way we don't have to fudge the nokey
ref count when a socket changes from nokey to normal.

Credits go to Mauricio Faria de Oliveira who diagnosed this bug
and sent a patch for it:

https://lore.kernel.org/linux-crypto/20200605161657.535043-1-mfo@canonical.com/

Reported-by: Brian Moyles 
Reported-by: Mauricio Faria de Oliveira 
Fixes: 37f96694cf73 ("crypto: af_alg - Use bh_lock_sock in...")
Cc: 
Signed-off-by: Herbert Xu 
Signed-off-by: Greg Kroah-Hartman

crypto: algif_skcipher - use ZERO_OR_NULL_PTR in skcipher_recvmsg_async

2020-03-11T06:51:19+00:00

Nowdays, we trigger a oops:
...
kasan: GPF could be caused by NULL-ptr deref or user memory accessgeneral protection fault: 0000 [#1] SMP KASAN
...
Call Trace:
 [] skcipher_recvmsg_async+0x3f1/0x1400 x86/../crypto/algif_skcipher.c:543
 [] skcipher_recvmsg+0x93/0x7f0 x86/../crypto/algif_skcipher.c:723
 [] sock_recvmsg_nosec x86/../net/socket.c:702 [inline]
 [] sock_recvmsg x86/../net/socket.c:710 [inline]
 [] sock_recvmsg+0x94/0xc0 x86/../net/socket.c:705
 [] sock_read_iter+0x27b/0x3a0 x86/../net/socket.c:787
 [] aio_run_iocb+0x21b/0x7a0 x86/../fs/aio.c:1520
 [] io_submit_one x86/../fs/aio.c:1630 [inline]
 [] do_io_submit+0x6b9/0x10b0 x86/../fs/aio.c:1688
 [] SYSC_io_submit x86/../fs/aio.c:1713 [inline]
 [] SyS_io_submit+0x2d/0x40 x86/../fs/aio.c:1710
 [] tracesys_phase2+0x90/0x95

In skcipher_recvmsg_async, we use '!sreq->tsg' to determine does we
calloc fail. However, kcalloc may return ZERO_SIZE_PTR, and with this,
the latter sg_init_table will trigger the bug. Fix it be use ZERO_OF_NULL_PTR.

This function was introduced with ' commit a596999b7ddf ("crypto:
algif - change algif_skcipher to be asynchronous")', and has been removed
with 'commit e870456d8e7c ("crypto: algif_skcipher - overhaul memory
management")'.

Reported-by: Hulk Robot 
Signed-off-by: yangerkun 
Signed-off-by: Greg Kroah-Hartman

crypto: AF_ALG - remove SGL terminator indicator when chaining

2017-09-27T09:00:14+00:00

Fixed differently upstream as commit 2d97591ef43d ("crypto: af_alg - consolidation of duplicate code")

The SGL is MAX_SGL_ENTS + 1 in size. The last SG entry is used for the
chaining and is properly updated with the sg_chain invocation. During
the filling-in of the initial SG entries, sg_mark_end is called for each
SG entry. This is appropriate as long as no additional SGL is chained
with the current SGL. However, when a new SGL is chained and the last
SG entry is updated with sg_chain, the last but one entry still contains
the end marker from the sg_mark_end. This end marker must be removed as
otherwise a walk of the chained SGLs will cause a NULL pointer
dereference at the last but one SG entry, because sg_next will return
NULL.

The patch only applies to all kernels up to and including 4.13. The
patch 2d97591ef43d0587be22ad1b0d758d6df4999a0b added to 4.14-rc1
introduced a complete new code base which addresses this bug in
a different way. Yet, that patch is too invasive for stable kernels
and was therefore not marked for stable.

Fixes: 8ff590903d5fc ("crypto: algif_skcipher - User-space interface for skcipher operations")
Signed-off-by: Stephan Mueller 
Acked-by: Herbert Xu 
Signed-off-by: Greg Kroah-Hartman

crypto: algif_skcipher - only call put_page on referenced and used pages

2017-09-07T06:34:10+00:00

commit 445a582738de6802669aeed9c33ca406c23c3b1f upstream.

For asynchronous operation, SGs are allocated without a page mapped to
them or with a page that is not used (ref-counted). If the SGL is freed,
the code must only call put_page for an SG if there was a page assigned
and ref-counted in the first place.

This fixes a kernel crash when using io_submit with more than one iocb
using the sendmsg and sendpage (vmsplice/splice) interface.

Signed-off-by: Stephan Mueller 
Signed-off-by: Herbert Xu 
Signed-off-by: Greg Kroah-Hartman

crypto: algif_skcipher - Do not set MAY_BACKLOG on the async path

2016-02-17T20:31:05+00:00

commit dad41997063723eaf5f77bc2015606a5a9bce320 upstream.

The async path cannot use MAY_BACKLOG because it is not meant to
block, which is what MAY_BACKLOG does.  On the other hand, both
the sync and async paths can make use of MAY_SLEEP.

Signed-off-by: Herbert Xu 
Signed-off-by: Greg Kroah-Hartman

crypto: algif_skcipher - Do not dereference ctx without socket lock

2016-02-17T20:31:05+00:00

commit 6454c2b83f719057069777132b13949e4c6b6350 upstream.

Any access to non-constant bits of the private context must be
done under the socket lock, in particular, this includes ctx->req.

This patch moves such accesses under the lock, and fetches the
tfm from the parent socket which is guaranteed to be constant,
rather than from ctx->req.

Signed-off-by: Herbert Xu 
Signed-off-by: Greg Kroah-Hartman

crypto: algif_skcipher - Do not assume that req is unchanged

2016-02-17T20:31:05+00:00

commit ec69bbfb9902c32a5c1492f2b1b8ad032a66d724 upstream.

The async path in algif_skcipher assumes that the crypto completion
function will be called with the original request.  This is not
necessarily the case.  In fact there is no need for this anyway
since we already embed information into the request with struct
skcipher_async_req.

This patch adds a pointer to that struct and then passes it as
the data to the callback function.

Signed-off-by: Herbert Xu 
Tested-by: Tadeusz Struk 
Signed-off-by: Greg Kroah-Hartman

crypto: algif_skcipher - sendmsg SG marking is off by one

2016-02-17T20:31:04+00:00

commit 202736d99b7f29279db9da61587f11a08a04a9c6 upstream.

We mark the end of the SG list in sendmsg and sendpage and unmark
it on the next send call.  Unfortunately the unmarking in sendmsg
is off-by-one, leading to an SG list that is too short.

Fixes: 0f477b655a52 ("crypto: algif - Mark sgl end at the end of data")
Signed-off-by: Herbert Xu 
Signed-off-by: Greg Kroah-Hartman

crypto: algif_skcipher - Load TX SG list after waiting

2016-02-17T20:31:04+00:00

commit 4f0414e54e4d1893c6f08260693f8ef84c929293 upstream.

We need to load the TX SG list in sendmsg(2) after waiting for
incoming data, not before.

Reported-by: Dmitry Vyukov 
Signed-off-by: Herbert Xu 
Tested-by: Dmitry Vyukov 
Signed-off-by: Greg Kroah-Hartman

crypto: algif_skcipher - Fix race condition in skcipher_check_key

2016-02-17T20:31:04+00:00

commit 1822793a523e5d5730b19cc21160ff1717421bc8 upstream.

We need to lock the child socket in skcipher_check_key as otherwise
two simultaneous calls can cause the parent socket to be freed.

Signed-off-by: Herbert Xu 
Signed-off-by: Greg Kroah-Hartman