Re: NFS/credentials leak in 2.6.29-rc1

["J. Bruce Fields" <bfields@fieldses.org>]

On Thu, Jan 22, 2009 at 07:08:33AM +0000, David Howells wrote:
> J. Bruce Fields <bfields@fieldses.org> 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.