On Wed, Jul 18, 2018 at 1:04 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
>
> int cachefiles_write_page(struct fscache_storage *op, struct page *page)
> {
> ...
> file = dentry_open(&path, O_RDWR | O_LARGEFILE, cache->cache_cred);
Ugh. So on the one hand, in this case I'd actually be more ok with the
whole "call_with_creds()" model, because open() is actually *supposed*
to use creds.
At the same time, my stronger reaction is that "it's actually better
to just pass down the creds as a pointer like we do".
So I think the real problem is simply that people use the current
creds without thinking about it. Often it's just hidden in a
"capable(CAP_SYS_ADMIN)" call or something like that, where people
don't even _think_ about the whole thing.
What I really think I'd prefer is to have some simple way to "poison"
current_cred(). It could be something as simple as a per-thread
counter, and we'd have current_cred() do
WARN_ON_ONCE(in_interrupt() || current->cred_poison);
and then read/write/open could just inc/dec the cred_poison counter
(when the debug option is set).
We wouldn't even need to catch all the odd cases. We'd only need to
inc/dec the poison counter in the normal read/write/open cases, not
even worrying about the odd special cases (like that
cachefiles_write_page() case).
Why? Because then we'd make sure the filesystems do the right thing,
and then cachefiles_write_page() etc wouldn't even have to worry,
because all the _common_ open cases have already tested that yes, it
uses the right cred pointer and doesn't try to get whatever "current"
happens to be.
So I'd really like to just fix the cases where we access the wrong
creds. And I'd be more than happy to add a few wrapper functions to
make us _see_ the cases where we do so.
What I don't like is the idea of trying to make bad creds accesses
"magically do the right thing".
See what I'm aiming for?
Linus
