linux-kernel - Re: [BUG] fs/dlm: A possible sleep-in-atomic bug in dlm_master

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: <20171007022611.GO21978@ZenIV.linux.org.uk>
Date:   Sat, 7 Oct 2017 03:26:11 +0100
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Jia-Ju Bai <baijiaju1990@....com>
Cc:     ccaulfie@...hat.com, teigland@...hat.com, cluster-devel@...hat.com,
        linux-kernel@...r.kernel.org
Subject: Re: [BUG] fs/dlm: A possible sleep-in-atomic bug in dlm_master_lookup

On Sat, Oct 07, 2017 at 09:59:41AM +0800, Jia-Ju Bai wrote:
> According to fs/dlm/lock.c, the kernel may sleep under a spinlock,
> and the function call path is:
> dlm_master_lookup (acquire the spinlock)
>   dlm_send_rcom_lookup_dump
>     create_rcom
>       dlm_lowcomms_get_buffer
>         nodeid2con
>           mutex_lock --> may sleep
> 
> This bug is found by my static analysis tool and my code review.

Umm...  dlm_master_lookup() locking is not nice, but to trigger that
you would need a combination of

* from_nodeid != our_nodeid (or we would've buggered off long before that point)
* dir_nodeid == our_nodeid
* failing dlm_search_rsb_tree(&ls->ls_rsbtbl[b].keep, name, len, &r)
(success would have the lock dropped)
* succeeding dlm_search_rsb_tree(&ls->ls_rsbtbl[b].toss, name, len, &r)
* from_master being true
* r->res_master_nodeid != from_nodeid and r->res_master_nodeid == our_nodeid
(the former is follows from the latter, actually)

The last one might or might not be impossible - I'm not familiar with dlm
guts, but it does have
                        log_error(ls, "from_master %d our_master", from_nodeid);
just before that call, so it's worth a further look.