[PATCH 10/16] scsi: arcmsr: Buffer overflow in arcmsr_iop_message_xfer()

[PATCH 10/16] scsi: arcmsr: Buffer overflow in arcmsr_iop_message_xfer()

[Philipp Hahn]

From: Dan Carpenter <dan.carpenter@oracle.com>

[ Upstream commit 7bc2b55a5c030685b399bb65b6baa9ccc3d1f167 ]

We need to put an upper bound on "user_len" so the memcpy() doesn't
overflow.

References: CVE-2016-7425
Cc: <stable@vger.kernel.org>
Reported-by: Marco Grassi <marco.gra@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Philipp Hahn <hahn@univention.de>
---
 drivers/scsi/arcmsr/arcmsr_hba.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 2926295..c9f87cd 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2300,7 +2300,8 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
 	}
 	case ARCMSR_MESSAGE_WRITE_WQBUFFER: {
 		unsigned char *ver_addr;
-		int32_t user_len, cnt2end;
+		uint32_t user_len;
+		int32_t cnt2end;
 		uint8_t *pQbuffer, *ptmpuserbuffer;
 		ver_addr = kmalloc(ARCMSR_API_DATA_BUFLEN, GFP_ATOMIC);
 		if (!ver_addr) {
@@ -2309,6 +2310,11 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
 		}
 		ptmpuserbuffer = ver_addr;
 		user_len = pcmdmessagefld->cmdmessage.Length;
+		if (user_len > ARCMSR_API_DATA_BUFLEN) {
+			retvalue = ARCMSR_MESSAGE_FAIL;
+			kfree(ver_addr);
+			goto message_out;
+		}
 		memcpy(ptmpuserbuffer,
 			pcmdmessagefld->messagedatabuffer, user_len);
 		spin_lock_irqsave(&acb->wqbuffer_lock, flags);
-- 
2.1.4

[PATCH 15/16] net: ping: check minimum size on ICMP header length

[Philipp Hahn]

From: Kees Cook <keescook@chromium.org>

[ Upstream commit 0eab121ef8750a5c8637d51534d5e9143fb0633f ]

Prior to commit c0371da6047a ("put iov_iter into msghdr") in v3.19, there
was no check that the iovec contained enough bytes for an ICMP header,
and the read loop would walk across neighboring stack contents. Since the
iov_iter conversion, bad arguments are noticed, but the returned error is
EFAULT. Returning EINVAL is a clearer error and also solves the problem
prior to v3.19.

This was found using trinity with KASAN on v3.18:

BUG: KASAN: stack-out-of-bounds in memcpy_fromiovec+0x60/0x114 at addr ffffffc071077da0
Read of size 8 by task trinity-c2/9623
page:ffffffbe034b9a08 count:0 mapcount:0 mapping:          (null) index:0x0
flags: 0x0()
page dumped because: kasan: bad access detected
CPU: 0 PID: 9623 Comm: trinity-c2 Tainted: G    BU         3.18.0-dirty #15
Hardware name: Google Tegra210 Smaug Rev 1,3+ (DT)
Call trace:
[<ffffffc000209c98>] dump_backtrace+0x0/0x1ac arch/arm64/kernel/traps.c:90
[<ffffffc000209e54>] show_stack+0x10/0x1c arch/arm64/kernel/traps.c:171
[<     inline     >] __dump_stack lib/dump_stack.c:15
[<ffffffc000f18dc4>] dump_stack+0x7c/0xd0 lib/dump_stack.c:50
[<     inline     >] print_address_description mm/kasan/report.c:147
[<     inline     >] kasan_report_error mm/kasan/report.c:236
[<ffffffc000373dcc>] kasan_report+0x380/0x4b8 mm/kasan/report.c:259
[<     inline     >] check_memory_region mm/kasan/kasan.c:264
[<ffffffc00037352c>] __asan_load8+0x20/0x70 mm/kasan/kasan.c:507
[<ffffffc0005b9624>] memcpy_fromiovec+0x5c/0x114 lib/iovec.c:15
[<     inline     >] memcpy_from_msg include/linux/skbuff.h:2667
[<ffffffc000ddeba0>] ping_common_sendmsg+0x50/0x108 net/ipv4/ping.c:674
[<ffffffc000dded30>] ping_v4_sendmsg+0xd8/0x698 net/ipv4/ping.c:714
[<ffffffc000dc91dc>] inet_sendmsg+0xe0/0x12c net/ipv4/af_inet.c:749
[<     inline     >] __sock_sendmsg_nosec net/socket.c:624
[<     inline     >] __sock_sendmsg net/socket.c:632
[<ffffffc000cab61c>] sock_sendmsg+0x124/0x164 net/socket.c:643
[<     inline     >] SYSC_sendto net/socket.c:1797
[<ffffffc000cad270>] SyS_sendto+0x178/0x1d8 net/socket.c:1761

CVE-2016-8399

Reported-by: Qidan He <i@flanker017.me>
Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind")
Cc: stable@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Philipp Hahn <hahn@univention.de>
---
 net/ipv4/ping.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index f6ee0d5..3dac3d4 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -659,6 +659,10 @@ int ping_common_sendmsg(int family, struct msghdr *msg, size_t len,
 	if (len > 0xFFFF)
 		return -EMSGSIZE;
 
+	/* Must have at least a full ICMP header. */
+	if (len < icmph_len)
+		return -EINVAL;
+
 	/*
 	 *	Check the flags.
 	 */
-- 
2.1.4

Re: [PATCH 10/16] scsi: arcmsr: Buffer overflow in arcmsr_iop_message_xfer()

[Greg KH]

On Wed, Dec 14, 2016 at 01:24:52PM +0100, Philipp Hahn wrote:
> From: Dan Carpenter <dan.carpenter@oracle.com>
> 
> [ Upstream commit 7bc2b55a5c030685b399bb65b6baa9ccc3d1f167 ]
> 
> We need to put an upper bound on "user_len" so the memcpy() doesn't
> overflow.
> 
> References: CVE-2016-7425
> Cc: <stable@vger.kernel.org>
> Reported-by: Marco Grassi <marco.gra@gmail.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Tomas Henzl <thenzl@redhat.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Philipp Hahn <hahn@univention.de>
> ---
>  drivers/scsi/arcmsr/arcmsr_hba.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 2926295..c9f87cd 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -2300,7 +2300,8 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
>  	}
>  	case ARCMSR_MESSAGE_WRITE_WQBUFFER: {
>  		unsigned char *ver_addr;
> -		int32_t user_len, cnt2end;
> +		uint32_t user_len;
> +		int32_t cnt2end;
>  		uint8_t *pQbuffer, *ptmpuserbuffer;
>  		ver_addr = kmalloc(ARCMSR_API_DATA_BUFLEN, GFP_ATOMIC);
>  		if (!ver_addr) {
> @@ -2309,6 +2310,11 @@ static int arcmsr_iop_message_xfer(struct AdapterControlBlock *acb,
>  		}
>  		ptmpuserbuffer = ver_addr;
>  		user_len = pcmdmessagefld->cmdmessage.Length;
> +		if (user_len > ARCMSR_API_DATA_BUFLEN) {
> +			retvalue = ARCMSR_MESSAGE_FAIL;
> +			kfree(ver_addr);
> +			goto message_out;
> +		}
>  		memcpy(ptmpuserbuffer,
>  			pcmdmessagefld->messagedatabuffer, user_len);
>  		spin_lock_irqsave(&acb->wqbuffer_lock, flags);
> -- 
> 2.1.4

I don't understand, what are we supposed to do with this patch?

confused,

greg k-h

Re: [PATCH 10/16] scsi: arcmsr: Buffer overflow in arcmsr_iop_message_xfer()

[Philipp Hahn]

Hi  Greg,
Sorry for the confusion. I have been working on our Kernel based on 4.1.36 and sent my work to Sasha for Integration into stable/4.1.y, but did not notice git sending those patches to all mentioned parties. Just ignore it.
Thank you for all your hard work.
Philipp
-- 
Univention GmbH

Re: [PATCH 10/16] scsi: arcmsr: Buffer overflow in arcmsr_iop_message_xfer()

[Greg KH]

On Thu, Dec 15, 2016 at 06:13:41AM +0100, Philipp Hahn wrote:
> Hi  Greg,
> Sorry for the confusion. I have been working on our Kernel based on 4.1.36 and sent my work to Sasha for Integration into stable/4.1.y, but did not notice git sending those patches to all mentioned parties. Just ignore it.

Are these patches also in 4.4-stable and 4.8-stable?

thanks,

greg k-h