linux-kernel - Re: [PATCH] dcache: fix dpath buffer corruption for too small buffers

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: <20140323043643.GA25566@ZenIV.linux.org.uk>
Date:	Sun, 23 Mar 2014 04:36:43 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Imre Deak <imre.deak@...el.com>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dcache: fix dpath buffer corruption for too small buffers

On Thu, Mar 13, 2014 at 01:29:46AM +0200, Imre Deak wrote:
> During dentry path lookups we can end up corrupting memory if the
> destination path buffer is too small. This is because prepend_path()
> and prepend() adjust the passed buffer length unconditionally, allowing
> for the buffer length to go negative. Then a later prepend_name() call
> will receive a negative length and convert this to unsigned before
> comparing it against the source string length leading to a possible
> memory corruption preceeding the destination buffer.
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 265e0ce..4015fd9 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2833,7 +2833,8 @@ static int prepend_name(char **buffer, int *buflen, struct qstr *name)
>  	u32 dlen = ACCESS_ONCE(name->len);
>  	char *p;
>  
> -	if (*buflen < dlen + 1)
> +	/* make sure we don't convert a negative value to unsigned int */
> +	if (*buflen < 0 || *buflen < dlen + 1)
>  		return -ENAMETOOLONG;
>  	*buflen -= dlen + 1;

It's much easier to fix, actually.  Look at the callers of prepend_name();
when it returns a negative value, they either discard the value left in
*buflen (__dentry_path()) or pass it back to their caller and return a
negative value themselves (prepend_path()).  The same is true for
prepend_path() (discared in __d_path(), d_absolute_path(), sys_getcwd();
passed to caller with negative return value in path_with_deleted()) and
path_with_deleted() (the only caller is d_path() and it always discards).

In other words, when prepend_name() returns -ENAMETOOLONG, it is free to
leave whatever it wants in *buflen.  So let's do what prepend() does -
subtract from *buflen, then check if the result has become negative.
Generates a better code than the original, actually...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/