| Message ID | 1384991735-18118-1-git-send-email-jayr@google.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Wed, Nov 20, 2013 at 03:55:35PM -0800, Junho Ryu wrote: > ext4_mb_put_pa should hold pa->pa_lock before accessing pa->pa_count. > While ext4_mb_use_preallocated checks pa->pa_deleted first and then > increments pa->count later, ext4_mb_put_pa decrements pa->pa_count > before holding pa->pa_lock and then sets pa->pa_deleted. > > * Free sequence > ext4_mb_put_pa (1): atomic_dec_and_test pa->pa_count > ext4_mb_put_pa (2): lock pa->pa_lock > ext4_mb_put_pa (3): check pa->pa_deleted > ext4_mb_put_pa (4): set pa->pa_deleted=1 > ext4_mb_put_pa (5): unlock pa->pa_lock > ext4_mb_put_pa (6): remove pa from a list > ext4_mb_pa_callback: free pa > > * Use sequence > ext4_mb_use_preallocated (1): iterate over preallocation > ext4_mb_use_preallocated (2): lock pa->pa_lock > ext4_mb_use_preallocated (3): check pa->pa_deleted > ext4_mb_use_preallocated (4): increase pa->pa_count > ext4_mb_use_preallocated (5): unlock pa->pa_lock > ext4_mb_release_context: access pa > > * Use-after-free sequence > [initial status] <pa->pa_deleted = 0, pa_count = 1> > ext4_mb_use_preallocated (1): iterate over preallocation > ext4_mb_use_preallocated (2): lock pa->pa_lock > ext4_mb_use_preallocated (3): check pa->pa_deleted > ext4_mb_put_pa (1): atomic_dec_and_test pa->pa_count > [pa_count decremented] <pa->pa_deleted = 0, pa_count = 0> > ext4_mb_use_preallocated (4): increase pa->pa_count > [pa_count incremented] <pa->pa_deleted = 0, pa_count = 1> > ext4_mb_use_preallocated (5): unlock pa->pa_lock > ext4_mb_put_pa (2): lock pa->pa_lock > ext4_mb_put_pa (3): check pa->pa_deleted > ext4_mb_put_pa (4): set pa->pa_deleted=1 > [race condition!] <pa->pa_deleted = 1, pa_count = 1> > ext4_mb_put_pa (5): unlock pa->pa_lock > ext4_mb_put_pa (6): remove pa from a list > ext4_mb_pa_callback: free pa > ext4_mb_release_context: access pa > > AddressSanitizer has detected use-after-free in ext4_mb_new_blocks > Bug report: http://goo.gl/rG1On3 > > Signed-off-by: Junho Ryu <jayr@google.com> Thanks, applied. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index a41e3ba..83403dd 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -3442,6 +3442,9 @@ static void ext4_mb_pa_callback(struct rcu_head *head) { struct ext4_prealloc_space *pa; pa = container_of(head, struct ext4_prealloc_space, u.pa_rcu); + + BUG_ON(atomic_read(&pa->pa_count)); + BUG_ON(pa->pa_deleted == 0); kmem_cache_free(ext4_pspace_cachep, pa); } @@ -3455,11 +3458,13 @@ static void ext4_mb_put_pa(struct ext4_allocation_context *ac, ext4_group_t grp; ext4_fsblk_t grp_blk; - if (!atomic_dec_and_test(&pa->pa_count) || pa->pa_free != 0) - return; - /* in this short window concurrent discard can set pa_deleted */ spin_lock(&pa->pa_lock); + if (!atomic_dec_and_test(&pa->pa_count) || pa->pa_free != 0) { + spin_unlock(&pa->pa_lock); + return; + } + if (pa->pa_deleted == 1) { spin_unlock(&pa->pa_lock); return;
ext4_mb_put_pa should hold pa->pa_lock before accessing pa->pa_count. While ext4_mb_use_preallocated checks pa->pa_deleted first and then increments pa->count later, ext4_mb_put_pa decrements pa->pa_count before holding pa->pa_lock and then sets pa->pa_deleted. * Free sequence ext4_mb_put_pa (1): atomic_dec_and_test pa->pa_count ext4_mb_put_pa (2): lock pa->pa_lock ext4_mb_put_pa (3): check pa->pa_deleted ext4_mb_put_pa (4): set pa->pa_deleted=1 ext4_mb_put_pa (5): unlock pa->pa_lock ext4_mb_put_pa (6): remove pa from a list ext4_mb_pa_callback: free pa * Use sequence ext4_mb_use_preallocated (1): iterate over preallocation ext4_mb_use_preallocated (2): lock pa->pa_lock ext4_mb_use_preallocated (3): check pa->pa_deleted ext4_mb_use_preallocated (4): increase pa->pa_count ext4_mb_use_preallocated (5): unlock pa->pa_lock ext4_mb_release_context: access pa * Use-after-free sequence [initial status] <pa->pa_deleted = 0, pa_count = 1> ext4_mb_use_preallocated (1): iterate over preallocation ext4_mb_use_preallocated (2): lock pa->pa_lock ext4_mb_use_preallocated (3): check pa->pa_deleted ext4_mb_put_pa (1): atomic_dec_and_test pa->pa_count [pa_count decremented] <pa->pa_deleted = 0, pa_count = 0> ext4_mb_use_preallocated (4): increase pa->pa_count [pa_count incremented] <pa->pa_deleted = 0, pa_count = 1> ext4_mb_use_preallocated (5): unlock pa->pa_lock ext4_mb_put_pa (2): lock pa->pa_lock ext4_mb_put_pa (3): check pa->pa_deleted ext4_mb_put_pa (4): set pa->pa_deleted=1 [race condition!] <pa->pa_deleted = 1, pa_count = 1> ext4_mb_put_pa (5): unlock pa->pa_lock ext4_mb_put_pa (6): remove pa from a list ext4_mb_pa_callback: free pa ext4_mb_release_context: access pa AddressSanitizer has detected use-after-free in ext4_mb_new_blocks Bug report: http://goo.gl/rG1On3 Signed-off-by: Junho Ryu <jayr@google.com> --- fs/ext4/mballoc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)