linux-kernel - Re: [PATCH 4.4 11/41] KEYS: fix writing past end of user-supplied buffer in keyring

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: <20171016181206.GC121701@google.com>
Date:   Mon, 16 Oct 2017 11:12:06 -0700
From:   Eric Biggers <ebiggers@...gle.com>
To:     Ben Hutchings <ben.hutchings@...ethink.co.uk>
Cc:     David Howells <dhowells@...hat.com>, stable@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4.4 11/41] KEYS: fix writing past end of user-supplied
 buffer in keyring_read()

On Mon, Oct 16, 2017 at 04:47:34PM +0100, Ben Hutchings wrote:
> On Tue, 2017-10-03 at 14:21 +0200, Greg Kroah-Hartman wrote:
> > 4.4-stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Eric Biggers <ebiggers@...gle.com>
> > 
> > commit e645016abc803dafc75e4b8f6e4118f088900ffb upstream.
> > 
> > Userspace can call keyctl_read() on a keyring to get the list of IDs of
> > keys in the keyring.  But if the user-supplied buffer is too small, the
> > kernel would write the full list anyway --- which will corrupt whatever
> > userspace memory happened to be past the end of the buffer.  Fix it by
> > only filling the space that is available.
> [...]
> 
> trusted_read() has the same bug.
> 
> Also, the comment above keyctl_read_key() says "return the amount of
> data that is available in the key, irrespective of how much we copied
> into the buffer."  All the other implementations of key_type::read seem
> to follow that, but this changes keyring_read() to return buflen in
> case of a truncated read.
> 

Hi Ben, thanks for pointing this out.  I had assumed the "obvious" semantics,
but it turns out that's not what's documented.  And actually the comment above
keyctl_read_key() might be wrong too, since the man page says that nothing is
copied if the buffer is too small...  David, which behavior was intended?  Some
of the ->read() methods will fill a too-small buffer, while others don't.

I think this bug isn't *too* bad since the short return will only be encountered
in cases where the kernel would have corrupted userspace memory before.  But
I'll send a patch to fix it.

And yes, trusted_read() is ignoring 'buflen' which is a bug too; I'll see if I
can fix that too...

Eric