linux-kernel - Re: deferring _

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: <20120625151812.GA16062@redhat.com>
Date:	Mon, 25 Jun 2012 17:18:12 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	". James Morris" <jmorris@...ei.org>,
	linux-security-module@...r.kernel.org,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: deferring __fput()

On 06/25, Al Viro wrote:
>
> On Sun, Jun 24, 2012 at 05:33:10PM +0200, Oleg Nesterov wrote:
> > No, we can't do this?
> >
> > OK, perhaps we can check something else instead of PF_EXITING.
> > But somehow we should ensuree that if task_work_add(twork) succeeds,
> > then twork->func() will be called. IOW, if task_work_add() races with
> > the exiting task, it should not succeed after exit_task_work().
>
> Hrm...  I still think that callers can bloody well check it themselves,

Why? I don't think this would be very convenient, and it is not easy
to avoid the races. Unless task == current.

OK, if task == current it can do the necessary checks, so we could add
"force" argument for fput(). But I agree, it would be better to avoid
this.

And since we want to move exit_task_work() after exit_fs() we can't
rely on PF_EXITING (unless we add "force").

> but anyway - we can add a new PF_... bit and have it set on kernel threads
> (all along)

Why? irq_thread() already uses task_work_add()...

> the real question is in locking
> and barriers needed there.  Suggestions?

Yes, we need more barries. Or, perhaps exit_task_work() should simply
take ->pi_lock unconditionally? I don't think additional STORE + mb()
is better.

And if it always takes ->pi_lock we do not need the new PF_ or something
else, exit_task_work() can set task->task_works = NO_MORE under ->pi_lock
(task_work_run() can check PF_EXITING), and task_work_add() ensures that
task_works != NO_MORE.

What do you think?

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/