From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932213AbdJXXTf (ORCPT ); Tue, 24 Oct 2017 19:19:35 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:45303 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751546AbdJXXTd (ORCPT ); Tue, 24 Oct 2017 19:19:33 -0400 X-Google-Smtp-Source: ABhQp+SuQOhkAnc9SK84Mny6bPbTg6/MHlGQFz3hBxQeGZL57lyqQPEWyIco1DCjZSAHtgJ4/Om0yQ== Date: Tue, 24 Oct 2017 16:19:27 -0700 From: Eric Biggers To: David Howells Cc: Ben Hutchings , stable@vger.kernel.org, Greg Kroah-Hartman , linux-kernel@vger.kernel.org Subject: Re: [PATCH 4.4 11/41] KEYS: fix writing past end of user-supplied buffer in keyring_read() Message-ID: <20171024231927.GA32603@google.com> References: <20171016181206.GC121701@google.com> <20171003114219.900672076@linuxfoundation.org> <20171003114220.627158480@linuxfoundation.org> <1508168854.22379.25.camel@codethink.co.uk> <10708.1508426843@warthog.procyon.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <10708.1508426843@warthog.procyon.org.uk> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 19, 2017 at 04:27:23PM +0100, David Howells wrote: > Eric Biggers 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