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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 19 Oct 2017 10:09:39 -0700
From:   Eric Biggers <ebiggers@...gle.com>
To:     David Howells <dhowells@...hat.com>
Cc:     Ben Hutchings <ben.hutchings@...ethink.co.uk>,
        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 Thu, Oct 19, 2017 at 04:27:23PM +0100, David Howells wrote:
> Eric Biggers <ebiggers@...gle.com> wrote:
> 
> > Hi Ben, thanks for pointing this out.  I had assumed the "obvious" semantics,
> > but it turns out that's not what's documented.
> 
> The manpage is correct.  keyctl_read_alloc() in libkeyutils relies on the
> behaviour documented there with respect to the full size of the data always
> being returned, even if the buffer was too small.
> 
> The keyring cannot be modified whilst it is being read, so that's not a
> concern.
> 
> keyctl_read_alloc() doesn't care if the buffer actually gets written to or
> not, but it's best to honour the manpage.
> 
> David

Do you mean we should also make it not copy anything at all if the buffer is too
small?  The man page says "If the buffer is too small ... no copy will take
place."  However, the in-kernel documentation contradicts it: "As much of the
data as can be fitted into the buffer will be copied to userspace if the buffer
pointer is not NULL.".  And meanwhile, the implementations of ->read() are split
between the two behaviors.

Eric