linux-kernel - Re: introduce probe_slab

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141022194228.GA10995@redhat.com>
Date:	Wed, 22 Oct 2014 21:42:28 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	David Miller <davem@...emloft.net>
Cc:	peterz@...radead.org, cl@...ux.com, tkhai@...dex.ru,
	ktkhai@...allels.com, linux-kernel@...r.kernel.org,
	mingo@...hat.com, vdavydov@...allels.com
Subject: Re: introduce probe_slab_address?

On 10/22, David Miller wrote:
>
> From: Oleg Nesterov <oleg@...hat.com>
> Date: Wed, 22 Oct 2014 20:14:12 +0200
>
> > So perhaps something like this makes sense?
> >
> > If some arch has problems with D-cache aliasing (because the freed page
> > can be already mapped by user-space or vmalloc'ed), it can redefine this
> > helper.
> >
> > Do you think we can use it to access rq->curr? (although let me repeat
> > that I won't really argue with SLAB_DESTROY_BY_RCU).
>
> Do we really need this?

Sorry, I was not clear. let me first explain why do we want this helper,
(although we can avoid it if we make task_struct_cachep SLAB_DESTROY_BY_RCU).

To simplify the discussion, lets ignore the actuall problem we are
trying to solve. Suppose that we want a trivial function, say,

	int pid_of_curr_task_on_cpu(int cpu)
	{
		 struct rq *rq = cpu_rq(cpu);
		 int pid;

		 rcu_read_lock();
		 pid = rq->curr->pid;
		 rcu_read_unlock();
	}

and we do not really care if it returns the wrong/random result.

The problem is that rcu_read_lock() can not help (unless we add
SLAB_DESTROY_BY_RCU), rq->curr is not protected by RCU. So rq->curr can
be freed. Again, we do not care, but CONFIG_DEBUG_PAGEALLOC can ummap
this page.

So we can use ifdef(CONFIG_DEBUG_PAGEALLOC) but it would be better to
have a helper to hide this ugliness, and (at least) slub can use it too.

Now the question: is this LOAD is safe in case when this (freed) page
already has another mapping? This is black magic to me, I do not know.
And Peter has some concerns.

And, say, copy_from_user_page() on sparc does

	flush_cache_page();
	memcpy();
	flush_ptrace_access();

Again, I simply do not know if this is relevant or not, probably this
is because "illegal alias" was created by kmap() which can use the
same vaddr to access different pages.

> We fully initialize and read from the area using the same virtual
> address, there is no possiblity for corruption from SLAB's
> perspective.
>
> It's when you store at vaddr X then read at vaddr Y and expect to see
> what you wrote at X that you have problems.
>
> That is very much not what is happening here.
>
> The lifetime is contained to SLAB's usage at one single virtual
> address.

OK, so iiuc this should be safe.

Thanks!

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/