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]

Message-ID: <1508923903.22379.77.camel@codethink.co.uk>
Date:   Wed, 25 Oct 2017 11:31:43 +0200
From:   Ben Hutchings <ben.hutchings@...ethink.co.uk>
To:     Eric Biggers <ebiggers@...gle.com>,
        David Howells <dhowells@...hat.com>
Cc:     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 Tue, 2017-10-24 at 16:19 -0700, Eric Biggers wrote:
[...]
> I'd like to fix this, but I still can't decide whether keyring_read() should
> fill the buffer or not.  In keyutils, keyctl_read_alloc() doesn't care, but
> keyctl_read() on a keyring is also called from dump_key_tree_aux().  And that
> *does* assume that the buffer was filled in the event of a short read ---
> although it can only happen if the keyring is added to concurrently, and even
> then it's still broken because dump_key_tree_aux() won't show everything in the
> keyring.
> 
> So do we really make it keep filling the buffer, even though that contradicts
> the man page?

I don't think any application is likely to care about this.  The
documentation should also be changed to say that if the buffer is too
small, "data may or may not be copied to the buffer" or "the contents
of the buffer are undefined".

Ben.

-- 
Ben Hutchings
Software Developer, Codethink Ltd.