linux-kernel - Re: [PATCH 0/1] aio: change exit_aio() to load mm->ioctx_table once and avoid rcu_read

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]

Date:	Tue, 29 Apr 2014 16:36:22 -0700
From:	Kent Overstreet <kmo@...erainc.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Benjamin LaHaise <bcrl@...ck.org>,
	Al Viro <viro@...iv.linux.org.uk>, linux-aio@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/1] aio: change exit_aio() to load mm->ioctx_table once
 and avoid rcu_read_lock()

On Tue, Apr 29, 2014 at 08:39:30PM +0200, Oleg Nesterov wrote:
> 1. We can read ->ioctx_table only once and we do not read rcu_read_lock()
>    or even rcu_dereference().
> 
>    This mm has no users, nobody else can play with ->ioctx_table. Otherwise
>    the code is buggy anyway, if we need rcu_read_lock() in a loop because
>    ->ioctx_table can be updated then kfree(table) is obviously wrong.
> 
> 2. Update the comment. "exit_mmap(mm) is coming" is the good reason to avoid
>    munmap(), but another reason is that we simply can't do vm_munmap() unless
>    current->mm == mm and this is not true in general, the caller is mmput().

I'm pretty sure you're analysis is all correct. IIRC there's other things in the
shutdown path we still have to be carefull with synchronization wise, but unless
I've forgotten something you're right about ioctx_table.

If I wrote that code (I'd have to check git blame), I'd say I just wrote it that
way because I prefer to be consistent about how things like RCU protected
pointers are accessed, even if it's not strictly necessary. Feels less tricky to
me. But I don't have a strong preference one way or the other.

w.r.t. ioctx->dead, yes, no need for it to be atomic. a regular int and xchg()
is probably more correct anyways since xchg() implies memory barriers and
atomic_xchg() does not.
--
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/