linux-stable.git/net/key, branch v3.18.97

af_key: fix buffer overread in parse_exthdrs()

2018-01-31T13:46:12+00:00

commit 4e765b4972af7b07adcb1feb16e7a525ce1f6b28 upstream.

If a message sent to a PF_KEY socket ended with an incomplete extension
header (fewer than 4 bytes remaining), then parse_exthdrs() read past
the end of the message, into uninitialized memory.  Fix it by returning
-EINVAL in this case.

Reproducer:

	#include 
	#include 
	#include 

	int main()
	{
		int sock = socket(PF_KEY, SOCK_RAW, PF_KEY_V2);
		char buf[17] = { 0 };
		struct sadb_msg *msg = (void *)buf;

		msg->sadb_msg_version = PF_KEY_V2;
		msg->sadb_msg_type = SADB_DELETE;
		msg->sadb_msg_len = 2;

		write(sock, buf, 17);
	}

Signed-off-by: Eric Biggers 
Signed-off-by: Steffen Klassert 
Signed-off-by: Greg Kroah-Hartman

af_key: fix buffer overread in verify_address_len()

2018-01-31T13:46:12+00:00

commit 06b335cb51af018d5feeff5dd4fd53847ddb675a upstream.

If a message sent to a PF_KEY socket ended with one of the extensions
that takes a 'struct sadb_address' but there were not enough bytes
remaining in the message for the ->sa_family member of the 'struct
sockaddr' which is supposed to follow, then verify_address_len() read
past the end of the message, into uninitialized memory.  Fix it by
returning -EINVAL in this case.

This bug was found using syzkaller with KMSAN.

Reproducer:

	#include 
	#include 
	#include 

	int main()
	{
		int sock = socket(PF_KEY, SOCK_RAW, PF_KEY_V2);
		char buf[24] = { 0 };
		struct sadb_msg *msg = (void *)buf;
		struct sadb_address *addr = (void *)(msg + 1);

		msg->sadb_msg_version = PF_KEY_V2;
		msg->sadb_msg_type = SADB_DELETE;
		msg->sadb_msg_len = 3;
		addr->sadb_address_len = 1;
		addr->sadb_address_exttype = SADB_EXT_ADDRESS_SRC;

		write(sock, buf, 24);
	}

Reported-by: Alexander Potapenko 
Signed-off-by: Eric Biggers 
Signed-off-by: Steffen Klassert 
Signed-off-by: Greg Kroah-Hartman

af_key: Add lock to key dump

2017-08-11T16:30:07+00:00

commit 89e357d83c06b6fac581c3ca7f0ee3ae7e67109e upstream.

A dump may come in the middle of another dump, modifying its dump
structure members. This race condition will result in NULL pointer
dereference in kernel. So add a lock to prevent that race.

Fixes: 83321d6b9872 ("[AF_KEY]: Dump SA/SP entries non-atomically")
Signed-off-by: Yuejie Shi 
Signed-off-by: Steffen Klassert 
Signed-off-by: Mark Salyzyn 
Signed-off-by: Greg Kroah-Hartman

af_key: Fix sadb_x_ipsecrequest parsing

2017-07-27T22:03:25+00:00

commit 096f41d3a8fcbb8dde7f71379b1ca85fe213eded upstream.

The parsing of sadb_x_ipsecrequest is broken in a number of ways.
First of all we're not verifying sadb_x_ipsecrequest_len.  This
is needed when the structure carries addresses at the end.  Worse
we don't even look at the length when we parse those optional
addresses.

The migration code had similar parsing code that's better but
it also has some deficiencies.  The length is overcounted first
of all as it includes the header itself.  It also fails to check
the length before dereferencing the sa_family field.

This patch fixes those problems in parse_sockaddr_pair and then
uses it in parse_ipsecrequest.

Reported-by: Andrey Konovalov 
Signed-off-by: Herbert Xu 
Signed-off-by: Steffen Klassert 
Signed-off-by: Greg Kroah-Hartman

xfrm: Oops on error in pfkey_msg2xfrm_state()

2017-07-05T12:35:14+00:00

commit 1e3d0c2c70cd3edb5deed186c5f5c75f2b84a633 upstream.

There are some missing error codes here so we accidentally return NULL
instead of an error pointer.  It results in a NULL pointer dereference.

Fixes: df71837d5024 ("[LSM-IPSec]: Security association restriction.")
Signed-off-by: Dan Carpenter 
Signed-off-by: Steffen Klassert 
Signed-off-by: Greg Kroah-Hartman

xfrm: NULL dereference on allocation failure

2017-07-05T12:35:14+00:00

commit e747f64336fc15e1c823344942923195b800aa1e upstream.

The default error code in pfkey_msg2xfrm_state() is -ENOBUFS.  We
added a new call to security_xfrm_state_alloc() which sets "err" to zero
so there several places where we can return ERR_PTR(0) if kmalloc()
fails.  The caller is expecting error pointers so it leads to a NULL
dereference.

Fixes: df71837d5024 ("[LSM-IPSec]: Security association restriction.")
Signed-off-by: Dan Carpenter 
Signed-off-by: Steffen Klassert 
Signed-off-by: Greg Kroah-Hartman

af_key: remove unnecessary break after return

2014-07-15T23:27:00+00:00

Signed-off-by: Fabian Frederick 
Signed-off-by: David S. Miller

af_key: Replace comma with semicolon

2014-05-31T00:48:58+00:00

This patch replaces a comma between expression statements by a semicolon.

A simplified version of the semantic patch that performs this
transformation is as follows:

// 
@r@
expression e1,e2,e;
type T;
identifier i;
@@

 e1
-,
+;
 e2;
// 

Signed-off-by: Himangi Saraogi 
Acked-by: Julia Lawall 
Signed-off-by: David S. Miller

xfrm: Remove useless xfrm_audit struct.

2014-04-23T06:21:04+00:00

Commit f1370cc4 "xfrm: Remove useless secid field from xfrm_audit." changed
"struct xfrm_audit" to have either
{ audit_get_loginuid(current) / audit_get_sessionid(current) } or
{ INVALID_UID / -1 } pair.

This means that we can represent "struct xfrm_audit" as "bool".
This patch replaces "struct xfrm_audit" argument with "bool".

Signed-off-by: Tetsuo Handa 
Signed-off-by: Steffen Klassert

xfrm: Remove useless secid field from xfrm_audit.

2014-04-22T08:47:53+00:00

It seems to me that commit ab5f5e8b "[XFRM]: xfrm audit calls" is doing
something strange at xfrm_audit_helper_usrinfo().
If secid != 0 && security_secid_to_secctx(secid) != 0, the caller calls
audit_log_task_context() which basically does
secid != 0 && security_secid_to_secctx(secid) == 0 case
except that secid is obtained from current thread's context.

Oh, what happens if secid passed to xfrm_audit_helper_usrinfo() was
obtained from other thread's context? It might audit current thread's
context rather than other thread's context if security_secid_to_secctx()
in xfrm_audit_helper_usrinfo() failed for some reason.

Then, are all the caller of xfrm_audit_helper_usrinfo() passing either
secid obtained from current thread's context or secid == 0?
It seems to me that they are.

If I didn't miss something, we don't need to pass secid to
xfrm_audit_helper_usrinfo() because audit_log_task_context() will
obtain secid from current thread's context.

Signed-off-by: Tetsuo Handa 
Signed-off-by: Steffen Klassert