Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: 11 Oct 2013 00:42:32 -0400
From: "George Spelvin" <linux@...izon.com>
To: ebiederm@...ssion.com, joe@...ches.com
Cc: akpm@...ux-foundation.org, dan.j.rosenberg@...il.com,
  eldad@...refinery.com, jgunthorpe@...idianresearch.com, jkosina@...e.cz,
  keescook@...omium.org, kernel-hardening@...ts.openwall.com,
  linux-kernel@...r.kernel.org, linux@...izon.com, rmallon@...il.com,
  viro@...iv.linux.org.uk
Subject: Re: [PATCH v3a] vsprintf: Check real user/group id for %pK

ebiederm@...ssion.com (Eric W. Biederman) wrote:
> Sigh.  This is all wrong.  The only correct thing to test is
> file->f_cred.  Aka the capabilities of the program that opened the
> file.
> 
> Which means that the interface to %pK in the case of kptr_restrict is
> broken as it has no way to be passed the information it needs to make
> a sensible decision.

I looked at the code, and pretty painful.  Certainly it's possible to
include a reference to the file (I was thinking of just the credentials,
actually) in the seq_file.  But getting that to the vsprintf.c code
(specifically, the pointer() function) is a PITA.

I'm willing to accept the currently proposed kludge as a "good enough"
approximation, as long as we're all agreed that using the credentials
at open() time would be The Right Thing, and hopefully someone will find
the round tuitts to implement that in future.

But in the meantime, "the perfect is the enemey of the good" is worth
remembering.

(An alternate implementation I've been thinking about would be to do
away with %pK, and instead have a "secret_ptr(p, seq->cred)" helper that
returned p or 0 depending on the credential.)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.