linux-kernel - Re: [V9fs-developer] [PATCH] Integer underflow in pdu

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 for Android: free password hash cracker in your pocket

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

Message-ID: <5B4565AA.6050309@huawei.com>
Date:   Wed, 11 Jul 2018 10:04:26 +0800
From:   jiangyiwen <jiangyiwen@...wei.com>
To:     Tomas Bortoli <tomasbortoli@...il.com>, <ericvh@...il.com>,
        <rminnich@...dia.gov>, <lucho@...kov.net>,
        Andrew Morton <akpm@...ux-foundation.org>
CC:     <linux-kernel@...r.kernel.org>, <syzkaller@...glegroups.com>,
        <v9fs-developer@...ts.sourceforge.net>, <davem@...emloft.net>
Subject: Re: [V9fs-developer] [PATCH] Integer underflow in pdu_read()

On 2018/7/10 3:26, Tomas Bortoli wrote:
> The pdu_read() function suffers from an integer underflow.
> When pdu->offset is greater than pdu->size, the length calculation will have
> a wrong result, resulting in an out-of-bound read.
> This patch modifies also pdu_write() in the same way to prevent the same
> issue from happening there and for consistency.

I guess this case may happened only when server send wrong size to
the client and then cause size < offset, or else I think this case
will not happen. Is it right? Or other cases?

In addition, the email should also send to andrew morton, because
9p maintainer already don't maintain the project, andrew can help
merge the patch.

> 
> Signed-off-by: Tomas Bortoli <tomasbortoli@...il.com>
> Reported-by: syzbot+65c6b72f284a39d416b4@...kaller.appspotmail.com
> ---
>  net/9p/protocol.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/net/9p/protocol.c b/net/9p/protocol.c
> index 931ea00c4fed..f1e2425f920b 100644
> --- a/net/9p/protocol.c
> +++ b/net/9p/protocol.c
> @@ -55,16 +55,20 @@ EXPORT_SYMBOL(p9stat_free);
>  
>  size_t pdu_read(struct p9_fcall *pdu, void *data, size_t size)
>  {
> -	size_t len = min(pdu->size - pdu->offset, size);
> -	memcpy(data, &pdu->sdata[pdu->offset], len);
> +	size_t len = pdu->offset > pdu->size ? 0 :
> +	 min(pdu->size - pdu->offset, size);

I suggest this add two *Tab* lens.

> +	if (len != 0)
> +		memcpy(data, &pdu->sdata[pdu->offset], len);
>  	pdu->offset += len;
>  	return size - len;
>  }
>  
>  static size_t pdu_write(struct p9_fcall *pdu, const void *data, size_t size)
>  {
> -	size_t len = min(pdu->capacity - pdu->size, size);
> -	memcpy(&pdu->sdata[pdu->size], data, len);
> +	size_t len = pdu->size > pdu->capacity ? 0 :
> +	 min(pdu->capacity - pdu->size, size);

The same as above.

> +	if (len != 0)
> +		memcpy(&pdu->sdata[pdu->size], data, len);
>  	pdu->size += len;
>  	return size - len;
>  }
>