Re: [PATCH 4.4 11/41] KEYS: fix writing past end of user-supplied buffer in keyring_read()

From: Eric Biggers
Date: Tue Oct 24 2017 - 19:19:49 EST


On Thu, Oct 19, 2017 at 04:27:23PM +0100, David Howells wrote:
> Eric Biggers <ebiggers@xxxxxxxxxx> 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

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?

Eric