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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 10 Dec 2021 10:03:37 +0800 From: Jia-Ju Bai <baijiaju1990@...il.com> To: "Theodore Y. Ts'o" <tytso@....edu> Cc: adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org, linux-kernel <linux-kernel@...r.kernel.org> Subject: Re: [BUG] fs: ext4: possible ABBA deadlock in ext4_inline_data_truncate() and ext4_punch_hole() On 2021/12/10 1:00, Theodore Y. Ts'o wrote: > On Thu, Dec 09, 2021 at 07:10:44PM +0800, Jia-Ju Bai wrote: >> Hello, >> >> My static analysis tool reports a possible ABBA deadlock in the ext4 module >> in Linux 5.10: >> >> ext4_inline_data_truncate() >> down_write(&EXT4_I(inode)->i_data_sem); --> Line 1895 (Lock A) >> ext4_xattr_ibody_get() >> ext4_xattr_inode_get() >> ext4_xattr_inode_iget() >> inode_lock(inode); --> Line 427 (Lock B) >> >> ext4_punch_hole() >> inode_lock(inode); --> Line 4018 (Lock B) >> ext4_update_disksize_before_punch() >> ext4_update_i_disksize() >> down_write(&EXT4_I(inode)->i_data_sem); --> Line 3248 (Lock A) >> >> When ext4_inline_data_truncate() and ext4_punch_hole() are concurrently >> executed, the deadlock can occur. >> >> I am not quite sure whether this possible deadlock is real and how to fix it >> if it is real. > Hi, > > Thanks for the report. I don't believe this is deadlock is possible, > because the first thing ext4_punch_hole() does is to check to see if > the inode has inline data --- and if so, it calls > ext4_convert_inline_data() to convert it to a normal file. In > ext4_convert_inline_data(), we take the xattr lock, and then do the > conversion, and then drop the xattr lock. So by the time > ext4_punch_hole() starts doing its work, the inode is not an inline > data file. > > In ext4_inline_data_truncate(), we take the xattr lock, and once we > have the xattr lock, we check to see if inode is still an inline data > file. If it has been converted, we then bail out. > > Hence, the ABBA deadlock that your static analysis tool has pointed > shouldn't happen in practice. Hi Ted, Thank you very much for the detailed explanation! I will improve my static analysis tool for this point. Best wishes, Jia-Ju Bai
Powered by blists - more mailing lists