From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: NFS/credentials leak in 2.6.29-rc1 Date: Thu, 22 Jan 2009 11:43:14 -0500 Message-ID: <20090122164314.GA13129@fieldses.org> References: <20090121223710.GF4295@fieldses.org> <20090120235341.GA29017@fieldses.org> <20090120114649.GA15832@ioremap.net> <20090120151125.GB24266@fieldses.org> <20090120152304.GA28592@ioremap.net> <21428.1232540589@redhat.com> <24419.1232608113@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Evgeniy Polyakov , Trond Myklebust , linux-fsdevel@vger.kernel.org To: David Howells Return-path: Received: from mail.fieldses.org ([141.211.133.115]:56748 "EHLO pickle.fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751258AbZAVQnU (ORCPT ); Thu, 22 Jan 2009 11:43:20 -0500 Content-Disposition: inline In-Reply-To: <24419.1232608113@redhat.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Jan 22, 2009 at 07:08:33AM +0000, David Howells wrote: > J. Bruce Fields wrote: > > > If the only difference is just whether it takes a reference on the > > passed-in cred it might be clearest just to write > > > > set_creds(new); > > > > or > > set_creds(get_creds(new)); > > > > depending on which you want? > > The former would be preferable, if it transfers the reference on the creds to > the task_struct, thus eliminating the need for a put_cred(). I think I was unclear--but I think we're agreeing anyway? I was proposing eliminating the two separate revert_creds() and override_creds() functions and instead using a single set_creds() that always consumes a reference to its argument, requiring the caller to explicitly get a reference (as in the second example above) when necessary. > > Is there a really big advantage to that? On the face of it it strikes > > me as a weird corner case that I'll trip over every time I look at this > > code. > > It'll remove a potential OOM condition. It's a minor optimisation, I think. OK. Let's keep things simple and set that idea aside for now; we've lived with the current groups_alloc(0) behavior for a while, and keeping it another kernel version or two can't be so bad. --b.