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

From: Eric Biggers
Date: Thu Oct 19 2017 - 13:09:52 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

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