On Tue, Jul 10, 2018 at 6:02 AM Jan Kara <jack@xxxxxxx> wrote:
> Currently chunk hash key (which is in fact pointer to the inode) is
> derived as chunk->mark.conn->obj. It is tricky to make this dereference
> reliable for hash table lookups only under RCU as mark can get detached
> from the connector and connector gets freed independently of the
> running lookup. Thus there is a possible use after free / NULL ptr
> dereference issue:
>
> CPU1 CPU2
> untag_chunk()
> ...
> audit_tree_lookup()
> list_for_each_entry_rcu(p, list, hash) {
> list_del_rcu(&chunk->hash);
> fsnotify_destroy_mark(entry);
> fsnotify_put_mark(entry)
> chunk_to_key(p)
> if (!chunk->mark.connector)
> ...
> hlist_del_init_rcu(&mark->obj_list);
> if (hlist_empty(&conn->list)) {
> inode = fsnotify_detach_connector_from_object(conn);
> mark->connector = NULL;
> ...
> frees connector from workqueue
> chunk->mark.connector->obj
>
> This race is probably impossible to hit in practice as the race window
> on CPU1 is very narrow and CPU2 has a lot of code to execute. Still it's
> better to have this fixed. Since the inode the chunk is attached to is
> constant during chunk's lifetime it is easy to cache the key in the
> chunk itself and thus avoid these issues.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> kernel/audit_tree.c | 27 ++++++++-------------------
> 1 file changed, 8 insertions(+), 19 deletions(-)
This looks okay to me.
--
paul moore
www.paul-moore.com
