netdev - Re: [GIT] Networking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20150209.205209.1524645061817000265.davem@davemloft.net>
Date:	Mon, 09 Feb 2015 20:52:09 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	torvalds@...ux-foundation.org
Cc:	viro@...iv.linux.org.uk, akpm@...ux-foundation.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT] Networking

From: Linus Torvalds <torvalds@...ux-foundation.org>
Date: Mon, 9 Feb 2015 20:37:13 -0800

> It's a NULL pointer derefernce (at offset 0x18) where the callchain
> looks like this:
> 
>   RIP: skcipher_recvmsg+0x360/0x410
>   Call Trace:
>      sock_read_iter+0xd0/0x120
>      new_sync_read+0x79/0xb0
>      __vfs_read+0x13/0x50
>      SyS_read+0x41/0x0b0
>      system_call_fastpath
> 
> which I assume is related to the iov_iter conversion.
> 
> That oops then is followed immediately by another that is a NULL
> pointer dereference in skcipher_sock_destruct, but the callchain for
> that is just the exit as part of killing of the original oops, so that
> second oops seems to be just a result of the first one.
> 
> I'm assuming the culrpit is 1d10eb2f156f ("crypto: switch
> af_alg_make_sg() to iov_iter") but haven't tested.

I think the handling of the 'used' local variable for function
skcipher_recvmsg() in that commit is suspect.

The problem is that if we go to the skcipher_wait_for_data() code
path, ctx->used is updated.

But the way skcipher_recvmsg() is was changed, that update won't
propagate into the caller because the old ctx->used value is cached in
the local 'used' variable.

The fix could be as simple as:

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 37110fd..4d1c315 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -444,6 +444,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
 			err = skcipher_wait_for_data(sk, flags);
 			if (err)
 				goto unlock;
+			used = ctx->used;
 		}
 
 		used = min_t(unsigned long, used, iov_iter_count(&msg->msg_iter));
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html