linux-stable.git/crypto/algif_hash.c, branch linux-3.10.y

crypto: algif_hash - avoid zero-sized array

2017-06-20T12:04:36+00:00

commit 6207119444595d287b1e9e83a2066c17209698f3 upstream.

With this reproducer:
  struct sockaddr_alg alg = {
          .salg_family = 0x26,
          .salg_type = "hash",
          .salg_feat = 0xf,
          .salg_mask = 0x5,
          .salg_name = "digest_null",
  };
  int sock, sock2;

  sock = socket(AF_ALG, SOCK_SEQPACKET, 0);
  bind(sock, (struct sockaddr *)&alg, sizeof(alg));
  sock2 = accept(sock, NULL, NULL);
  setsockopt(sock, SOL_ALG, ALG_SET_KEY, "\x9b\xca", 2);
  accept(sock2, NULL, NULL);

==== 8< ======== 8< ======== 8< ======== 8< ====

one can immediatelly see an UBSAN warning:
UBSAN: Undefined behaviour in crypto/algif_hash.c:187:7
variable length array bound value 0 <= 0
CPU: 0 PID: 15949 Comm: syz-executor Tainted: G            E      4.4.30-0-default #1
...
Call Trace:
...
 [] ? __ubsan_handle_vla_bound_not_positive+0x13d/0x188
 [] ? __ubsan_handle_out_of_bounds+0x1bc/0x1bc
 [] ? hash_accept+0x5bd/0x7d0 [algif_hash]
 [] ? hash_accept_nokey+0x3f/0x51 [algif_hash]
 [] ? hash_accept_parent_nokey+0x4a0/0x4a0 [algif_hash]
 [] ? SyS_accept+0x2b/0x40

It is a correct warning, as hash state is propagated to accept as zero,
but creating a zero-length variable array is not allowed in C.

Fix this as proposed by Herbert -- do "?: 1" on that site. No sizeof or
similar happens in the code there, so we just allocate one byte even
though we do not use the array.

Signed-off-by: Jiri Slaby 
Cc: Herbert Xu 
Cc: "David S. Miller"  (maintainer:CRYPTO API)
Reported-by: Sasha Levin 
Signed-off-by: Herbert Xu 
Cc: Arnd Bergmann 
Signed-off-by: Jiri Slaby 
Signed-off-by: Willy Tarreau

crypto: algif_hash - Fix race condition in hash_check_key

2017-02-06T08:04:07+00:00

commit ad46d7e33219218605ea619e32553daf4f346b9f upstream.

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

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu 
Signed-off-by: Andrey Ryabinin 
Signed-off-by: Willy Tarreau

crypto: algif_hash - Remove custom release parent function

2017-02-06T08:04:07+00:00

commit f1d84af1835846a5a2b827382c5848faf2bb0e75 upstream.

This patch removes the custom release parent function as the
generic af_alg_release_parent now works for nokey sockets too.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu 
Signed-off-by: Andrey Ryabinin 
Signed-off-by: Willy Tarreau

crypto: algif_hash - Require setkey before accept(2)

2017-02-06T08:04:07+00:00

commit 6de62f15b581f920ade22d758f4c338311c2f0d4 upstream.

Hash implementations that require a key may crash if you use
them without setting a key.  This patch adds the necessary checks
so that if you do attempt to use them without a key that we return
-ENOKEY instead of proceeding.

This patch also adds a compatibility path to support old applications
that do acept(2) before setkey.

Cc: stable@vger.kernel.org
Signed-off-by: Herbert Xu 
Signed-off-by: Andrey Ryabinin 
Signed-off-by: Willy Tarreau

crypto: algif_hash - wait for crypto_ahash_init() to complete

2016-02-19T22:22:41+00:00

commit fe09786178f9df713a4b2dd6b93c0a722346bf5e upstream.

hash_sendmsg/sendpage() need to wait for the completion
of crypto_ahash_init() otherwise it can cause panic.

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

crypto: algif_hash - Only export and import on sockets with data

2016-02-19T22:22:37+00:00

commit 4afa5f9617927453ac04b24b584f6c718dfb4f45 upstream.

The hash_accept call fails to work on sockets that have not received
any data.  For some algorithm implementations it may cause crashes.

This patch fixes this by ensuring that we only export and import on
sockets that have received data.

Reported-by: Harsh Jain 
Signed-off-by: Herbert Xu 
Tested-by: Stephan Mueller 
Signed-off-by: Greg Kroah-Hartman

Revert "net: update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST"

2013-12-20T15:45:12+00:00

It turns out that commit: d3f7d56a7a4671d395e8af87071068a195257bf6 was
applied to the tree twice, which didn't hurt anything, but it's good to
fix this up.

Reported-by: Veaceslav Falico 

Cc: David S. Miller 
Cc: Eric Dumazet 
Cc: Richard Weinberger 
Cc: Shawn Landden 
Cc: Tom Herbert 
Signed-off-by: Greg Kroah-Hartman

net: update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST

2013-12-12T06:36:27+00:00

commit d3f7d56a7a4671d395e8af87071068a195257bf6 upstream.

Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
added an internal flag MSG_SENDPAGE_NOTLAST, similar to
MSG_MORE.

algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
and need to see the new flag as identical to MSG_MORE.

This fixes sendfile() on AF_ALG.

v3: also fix udp

Cc: Tom Herbert 
Cc: Eric Dumazet 
Cc: David S. Miller 
Reported-and-tested-by: Shawn Landden 
Original-patch: Richard Weinberger 
Signed-off-by: Shawn Landden 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

net: update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST

2013-12-08T15:29:26+00:00

[ Upstream commit d3f7d56a7a4671d395e8af87071068a195257bf6 ]

Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
added an internal flag MSG_SENDPAGE_NOTLAST, similar to
MSG_MORE.

algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
and need to see the new flag as identical to MSG_MORE.

This fixes sendfile() on AF_ALG.

v3: also fix udp

Reported-and-tested-by: Shawn Landden 
Cc: Tom Herbert 
Cc: Eric Dumazet 
Cc: David S. Miller 
Original-patch: Richard Weinberger 
Signed-off-by: Shawn Landden 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

net: rework recvmsg handler msg_name and msg_namelen logic

2013-12-08T15:29:25+00:00

[ Upstream commit f3d3342602f8bcbf37d7c46641cb9bca7618eb1c ]

This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
to return msg_name to the user.

This prevents numerous uninitialized memory leaks we had in the
recvmsg handlers and makes it harder for new code to accidentally leak
uninitialized memory.

Optimize for the case recvfrom is called with NULL as address. We don't
need to copy the address at all, so set it to NULL before invoking the
recvmsg handler. We can do so, because all the recvmsg handlers must
cope with the case a plain read() is called on them. read() also sets
msg_name to NULL.

Also document these changes in include/linux/net.h as suggested by David
Miller.

Changes since RFC:

Set msg->msg_name = NULL if user specified a NULL in msg_name but had a
non-null msg_namelen in verify_iovec/verify_compat_iovec. This doesn't
affect sendto as it would bail out earlier while trying to copy-in the
address. It also more naturally reflects the logic by the callers of
verify_iovec.

With this change in place I could remove "
if (!uaddr || msg_sys->msg_namelen == 0)
	msg->msg_name = NULL
".

This change does not alter the user visible error logic as we ignore
msg_namelen as long as msg_name is NULL.

Also remove two unnecessary curly brackets in ___sys_recvmsg and change
comments to netdev style.

Cc: David Miller 
Suggested-by: Eric Dumazet 
Signed-off-by: Hannes Frederic Sowa 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman