linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@google.com>
To: David Howells <dhowells@redhat.com>
Cc: Ben Hutchings <ben.hutchings@codethink.co.uk>,
	stable@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4.4 11/41] KEYS: fix writing past end of user-supplied buffer in keyring_read()
Date: Tue, 24 Oct 2017 16:19:27 -0700	[thread overview]
Message-ID: <20171024231927.GA32603@google.com> (raw)
In-Reply-To: <10708.1508426843@warthog.procyon.org.uk>

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

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

  parent reply	other threads:[~2017-10-24 23:19 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-03 12:21 [PATCH 4.4 00/41] 4.4.90-stable review Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 01/41] cifs: release auth_key.response for reconnect Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 02/41] mac80211: flush hw_roc_start work before cancelling the ROC Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 03/41] KVM: PPC: Book3S: Fix race and leak in kvm_vm_ioctl_create_spapr_tce() Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 04/41] tracing: Fix trace_pipe behavior for instance traces Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 05/41] tracing: Erase irqsoff trace with empty write Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 06/41] md/raid5: fix a race condition in stripe batch Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 07/41] md/raid5: preserve STRIPE_ON_UNPLUG_LIST in break_stripe_batch_list Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 08/41] scsi: scsi_transport_iscsi: fix the issue that iscsi_if_rx doesnt parse nlmsg properly Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 09/41] crypto: talitos - Dont provide setkey for non hmac hashing algs Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 10/41] crypto: talitos - fix sha224 Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 11/41] KEYS: fix writing past end of user-supplied buffer in keyring_read() Greg Kroah-Hartman
2017-10-16 15:47   ` Ben Hutchings
2017-10-16 18:12     ` Eric Biggers
2017-10-19 15:27       ` David Howells
2017-10-19 17:09         ` Eric Biggers
2017-10-24 23:19         ` Eric Biggers [this message]
2017-10-25  9:31           ` Ben Hutchings
2017-11-01 15:24             ` David Howells
2017-10-03 12:21 ` [PATCH 4.4 12/41] KEYS: prevent creating a different users keyrings Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 13/41] KEYS: prevent KEYCTL_READ on negative key Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 14/41] powerpc/pseries: Fix parent_dn reference leak in add_dt_node() Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 15/41] Fix SMB3.1.1 guest authentication to Samba Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 16/41] SMB: Validate negotiate (to protect against downgrade) even if signing off Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 17/41] SMB3: Dont ignore O_SYNC/O_DSYNC and O_DIRECT flags Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 18/41] vfs: Return -ENXIO for negative SEEK_HOLE / SEEK_DATA offsets Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 19/41] nl80211: check for the required netlink attributes presence Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 20/41] bsg-lib: dont free job in bsg_prepare_job Greg Kroah-Hartman
2017-10-16 16:32   ` Ben Hutchings
2017-10-03 12:21 ` [PATCH 4.4 21/41] seccomp: fix the usage of get/put_seccomp_filter() in seccomp_get_filter() Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 22/41] arm64: Make sure SPsel is always set Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 23/41] arm64: fault: Route pte translation faults via do_translation_fault Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 25/41] kvm: nVMX: Dont allow L2 to access the hardware CR8 Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 26/41] PCI: Fix race condition with driver_override Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 27/41] btrfs: fix NULL pointer dereference from free_reloc_roots() Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 28/41] btrfs: propagate error to btrfs_cmp_data_prepare caller Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 29/41] btrfs: prevent to set invalid default subvolid Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 30/41] x86/fpu: Dont let userspace set bogus xcomp_bv Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 31/41] gfs2: Fix debugfs glocks dump Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 32/41] timer/sysclt: Restrict timer migration sysctl values to 0 and 1 Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 35/41] cxl: Fix driver use count Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 36/41] dmaengine: mmp-pdma: add number of requestors Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 37/41] ARM: pxa: add the number of DMA requestor lines Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 38/41] ARM: pxa: fix " Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 39/41] KVM: VMX: use cmpxchg64 Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 40/41] video: fbdev: aty: do not leak uninitialized padding in clk to userspace Greg Kroah-Hartman
2017-10-03 12:21 ` [PATCH 4.4 41/41] swiotlb-xen: implement xen_swiotlb_dma_mmap callback Greg Kroah-Hartman
2017-10-03 19:26 ` [PATCH 4.4 00/41] 4.4.90-stable review Shuah Khan
2017-10-03 20:30 ` Tom Gall
2017-10-04  7:55   ` Greg Kroah-Hartman
2017-10-04  8:29     ` Sumit Semwal
2017-10-03 20:41 ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171024231927.GA32603@google.com \
    --to=ebiggers@google.com \
    --cc=ben.hutchings@codethink.co.uk \
    --cc=dhowells@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).