[PATCH 04/10] audit: Embed key into chunk

Paul Moore paul at paul-moore.com
Fri Jul 27 04:47:17 UTC 2018


On Tue, Jul 10, 2018 at 6:02 AM Jan Kara <jack at suse.cz> 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 at suse.cz>
> ---
>  kernel/audit_tree.c | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)

This looks okay to me.

--
paul moore
www.paul-moore.com




More information about the Linux-audit mailing list