Skip to content

Commit 5781ac2

Browse files
tytsogregkh
tytso
authored and
gregkh
committed
ext4: fix deadlock between inline_data and ext4_expand_extra_isize_ea()
commit c755e251357a0cee0679081f08c3f4ba797a8009 upstream. The xattr_sem deadlock problems fixed in commit 2e81a4eeedca: "ext4: avoid deadlock when expanding inode size" didn't include the use of xattr_sem in fs/ext4/inline.c. With the addition of project quota which added a new extra inode field, this exposed deadlocks in the inline_data code similar to the ones fixed by 2e81a4eeedca. The deadlock can be reproduced via: dmesg -n 7 mke2fs -t ext4 -O inline_data -Fq -I 256 /dev/vdc 32768 mount -t ext4 -o debug_want_extra_isize=24 /dev/vdc /vdc mkdir /vdc/a umount /vdc mount -t ext4 /dev/vdc /vdc echo foo > /vdc/a/foo and looks like this: [ 11.158815] [ 11.160276] ============================================= [ 11.161960] [ INFO: possible recursive locking detected ] [ 11.161960] 4.10.0-rc3-00015-g011b30a8a3cf #160 Tainted: G W [ 11.161960] --------------------------------------------- [ 11.161960] bash/2519 is trying to acquire lock: [ 11.161960] (&ei->xattr_sem){++++..}, at: [<c1225a4b>] ext4_expand_extra_isize_ea+0x3d/0x4cd [ 11.161960] [ 11.161960] but task is already holding lock: [ 11.161960] (&ei->xattr_sem){++++..}, at: [<c1227941>] ext4_try_add_inline_entry+0x3a/0x152 [ 11.161960] [ 11.161960] other info that might help us debug this: [ 11.161960] Possible unsafe locking scenario: [ 11.161960] [ 11.161960] CPU0 [ 11.161960] ---- [ 11.161960] lock(&ei->xattr_sem); [ 11.161960] lock(&ei->xattr_sem); [ 11.161960] [ 11.161960] *** DEADLOCK *** [ 11.161960] [ 11.161960] May be due to missing lock nesting notation [ 11.161960] [ 11.161960] 4 locks held by bash/2519: [ 11.161960] #0: (sb_writers#3){.+.+.+}, at: [<c11a2414>] mnt_want_write+0x1e/0x3e [ 11.161960] #1: (&type->i_mutex_dir_key){++++++}, at: [<c119508b>] path_openat+0x338/0x67a [ 11.161960] AOSP-JF-MM#2: (jbd2_handle){++++..}, at: [<c123314a>] start_this_handle+0x582/0x622 [ 11.161960] CyanogenMod#3: (&ei->xattr_sem){++++..}, at: [<c1227941>] ext4_try_add_inline_entry+0x3a/0x152 [ 11.161960] [ 11.161960] stack backtrace: [ 11.161960] CPU: 0 PID: 2519 Comm: bash Tainted: G W 4.10.0-rc3-00015-g011b30a8a3cf #160 [ 11.161960] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.1-1 04/01/2014 [ 11.161960] Call Trace: [ 11.161960] dump_stack+0x72/0xa3 [ 11.161960] __lock_acquire+0xb7c/0xcb9 [ 11.161960] ? kvm_clock_read+0x1f/0x29 [ 11.161960] ? __lock_is_held+0x36/0x66 [ 11.161960] ? __lock_is_held+0x36/0x66 [ 11.161960] lock_acquire+0x106/0x18a [ 11.161960] ? ext4_expand_extra_isize_ea+0x3d/0x4cd [ 11.161960] down_write+0x39/0x72 [ 11.161960] ? ext4_expand_extra_isize_ea+0x3d/0x4cd [ 11.161960] ext4_expand_extra_isize_ea+0x3d/0x4cd [ 11.161960] ? _raw_read_unlock+0x22/0x2c [ 11.161960] ? jbd2_journal_extend+0x1e2/0x262 [ 11.161960] ? __ext4_journal_get_write_access+0x3d/0x60 [ 11.161960] ext4_mark_inode_dirty+0x17d/0x26d [ 11.161960] ? ext4_add_dirent_to_inline.isra.12+0xa5/0xb2 [ 11.161960] ext4_add_dirent_to_inline.isra.12+0xa5/0xb2 [ 11.161960] ext4_try_add_inline_entry+0x69/0x152 [ 11.161960] ext4_add_entry+0xa3/0x848 [ 11.161960] ? __brelse+0x14/0x2f [ 11.161960] ? _raw_spin_unlock_irqrestore+0x44/0x4f [ 11.161960] ext4_add_nondir+0x17/0x5b [ 11.161960] ext4_create+0xcf/0x133 [ 11.161960] ? ext4_mknod+0x12f/0x12f [ 11.161960] lookup_open+0x39e/0x3fb [ 11.161960] ? __wake_up+0x1a/0x40 [ 11.161960] ? lock_acquire+0x11e/0x18a [ 11.161960] path_openat+0x35c/0x67a [ 11.161960] ? sched_clock_cpu+0xd7/0xf2 [ 11.161960] do_filp_open+0x36/0x7c [ 11.161960] ? _raw_spin_unlock+0x22/0x2c [ 11.161960] ? __alloc_fd+0x169/0x173 [ 11.161960] do_sys_open+0x59/0xcc [ 11.161960] SyS_open+0x1d/0x1f [ 11.161960] do_int80_syscall_32+0x4f/0x61 [ 11.161960] entry_INT80_32+0x2f/0x2f [ 11.161960] EIP: 0xb76ad469 [ 11.161960] EFLAGS: 00000286 CPU: 0 [ 11.161960] EAX: ffffffda EBX: 08168ac8 ECX: 00008241 EDX: 000001b6 [ 11.161960] ESI: b75e46bc EDI: b7755000 EBP: bfbdb108 ESP: bfbdafc0 [ 11.161960] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b Cc: stable@vger.kernel.org # 3.10 (requires 2e81a4eeedca as a prereq) Reported-by: George Spelvin <linux@sciencehorizons.net> Signed-off-by: Theodore Ts'o <tytso@mit.edu> Signed-off-by: Harsh Shandilya <harsh@prjkt.io> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 48bb699 commit 5781ac2

3 files changed

Lines changed: 74 additions & 54 deletions

File tree

‎fs/ext4/inline.c‎

Lines changed: 30 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ static int ext4_update_inline_data(handle_t *handle, struct inode *inode,
374374
static int ext4_prepare_inline_data(handle_t *handle, struct inode *inode,
375375
unsigned int len)
376376
{
377-
int ret, size;
377+
int ret, size, no_expand;
378378
struct ext4_inode_info *ei = EXT4_I(inode);
379379

380380
if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
@@ -384,15 +384,14 @@ static int ext4_prepare_inline_data(handle_t *handle, struct inode *inode,
384384
if (size < len)
385385
return -ENOSPC;
386386

387-
down_write(&EXT4_I(inode)->xattr_sem);
387+
ext4_write_lock_xattr(inode, &no_expand);
388388

389389
if (ei->i_inline_off)
390390
ret = ext4_update_inline_data(handle, inode, len);
391391
else
392392
ret = ext4_create_inline_data(handle, inode, len);
393393

394-
up_write(&EXT4_I(inode)->xattr_sem);
395-
394+
ext4_write_unlock_xattr(inode, &no_expand);
396395
return ret;
397396
}
398397

@@ -522,7 +521,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
522521
struct inode *inode,
523522
unsigned flags)
524523
{
525-
int ret, needed_blocks;
524+
int ret, needed_blocks, no_expand;
526525
handle_t *handle = NULL;
527526
int retries = 0, sem_held = 0;
528527
struct page *page = NULL;
@@ -562,7 +561,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
562561
goto out;
563562
}
564563

565-
down_write(&EXT4_I(inode)->xattr_sem);
564+
ext4_write_lock_xattr(inode, &no_expand);
566565
sem_held = 1;
567566
/* If some one has already done this for us, just exit. */
568567
if (!ext4_has_inline_data(inode)) {
@@ -598,7 +597,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
598597
page_cache_release(page);
599598
page = NULL;
600599
ext4_orphan_add(handle, inode);
601-
up_write(&EXT4_I(inode)->xattr_sem);
600+
ext4_write_unlock_xattr(inode, &no_expand);
602601
sem_held = 0;
603602
ext4_journal_stop(handle);
604603
handle = NULL;
@@ -624,7 +623,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
624623
page_cache_release(page);
625624
}
626625
if (sem_held)
627-
up_write(&EXT4_I(inode)->xattr_sem);
626+
ext4_write_unlock_xattr(inode, &no_expand);
628627
if (handle)
629628
ext4_journal_stop(handle);
630629
brelse(iloc.bh);
@@ -717,7 +716,7 @@ int ext4_try_to_write_inline_data(struct address_space *mapping,
717716
int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
718717
unsigned copied, struct page *page)
719718
{
720-
int ret;
719+
int ret, no_expand;
721720
void *kaddr;
722721
struct ext4_iloc iloc;
723722

@@ -735,7 +734,7 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
735734
goto out;
736735
}
737736

738-
down_write(&EXT4_I(inode)->xattr_sem);
737+
ext4_write_lock_xattr(inode, &no_expand);
739738
BUG_ON(!ext4_has_inline_data(inode));
740739

741740
kaddr = kmap_atomic(page);
@@ -745,7 +744,7 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
745744
/* clear page dirty so that writepages wouldn't work for us. */
746745
ClearPageDirty(page);
747746

748-
up_write(&EXT4_I(inode)->xattr_sem);
747+
ext4_write_unlock_xattr(inode, &no_expand);
749748
brelse(iloc.bh);
750749
out:
751750
return copied;
@@ -756,7 +755,7 @@ ext4_journalled_write_inline_data(struct inode *inode,
756755
unsigned len,
757756
struct page *page)
758757
{
759-
int ret;
758+
int ret, no_expand;
760759
void *kaddr;
761760
struct ext4_iloc iloc;
762761

@@ -766,11 +765,11 @@ ext4_journalled_write_inline_data(struct inode *inode,
766765
return NULL;
767766
}
768767

769-
down_write(&EXT4_I(inode)->xattr_sem);
768+
ext4_write_lock_xattr(inode, &no_expand);
770769
kaddr = kmap_atomic(page);
771770
ext4_write_inline_data(inode, &iloc, kaddr, 0, len);
772771
kunmap_atomic(kaddr);
773-
up_write(&EXT4_I(inode)->xattr_sem);
772+
ext4_write_unlock_xattr(inode, &no_expand);
774773

775774
return iloc.bh;
776775
}
@@ -1245,7 +1244,7 @@ static int ext4_convert_inline_data_nolock(handle_t *handle,
12451244
int ext4_try_add_inline_entry(handle_t *handle, struct dentry *dentry,
12461245
struct inode *inode)
12471246
{
1248-
int ret, inline_size;
1247+
int ret, inline_size, no_expand;
12491248
void *inline_start;
12501249
struct ext4_iloc iloc;
12511250
struct inode *dir = dentry->d_parent->d_inode;
@@ -1254,7 +1253,7 @@ int ext4_try_add_inline_entry(handle_t *handle, struct dentry *dentry,
12541253
if (ret)
12551254
return ret;
12561255

1257-
down_write(&EXT4_I(dir)->xattr_sem);
1256+
ext4_write_lock_xattr(dir, &no_expand);
12581257
if (!ext4_has_inline_data(dir))
12591258
goto out;
12601259

@@ -1299,7 +1298,7 @@ int ext4_try_add_inline_entry(handle_t *handle, struct dentry *dentry,
12991298

13001299
out:
13011300
ext4_mark_inode_dirty(handle, dir);
1302-
up_write(&EXT4_I(dir)->xattr_sem);
1301+
ext4_write_unlock_xattr(dir, &no_expand);
13031302
brelse(iloc.bh);
13041303
return ret;
13051304
}
@@ -1655,15 +1654,15 @@ int ext4_delete_inline_entry(handle_t *handle,
16551654
struct buffer_head *bh,
16561655
int *has_inline_data)
16571656
{
1658-
int err, inline_size;
1657+
int err, inline_size, no_expand;
16591658
struct ext4_iloc iloc;
16601659
void *inline_start;
16611660

16621661
err = ext4_get_inode_loc(dir, &iloc);
16631662
if (err)
16641663
return err;
16651664

1666-
down_write(&EXT4_I(dir)->xattr_sem);
1665+
ext4_write_lock_xattr(dir, &no_expand);
16671666
if (!ext4_has_inline_data(dir)) {
16681667
*has_inline_data = 0;
16691668
goto out;
@@ -1698,7 +1697,7 @@ int ext4_delete_inline_entry(handle_t *handle,
16981697

16991698
ext4_show_inline_dir(dir, iloc.bh, inline_start, inline_size);
17001699
out:
1701-
up_write(&EXT4_I(dir)->xattr_sem);
1700+
ext4_write_unlock_xattr(dir, &no_expand);
17021701
brelse(iloc.bh);
17031702
if (err != -ENOENT)
17041703
ext4_std_error(dir->i_sb, err);
@@ -1797,11 +1796,11 @@ int empty_inline_dir(struct inode *dir, int *has_inline_data)
17971796

17981797
int ext4_destroy_inline_data(handle_t *handle, struct inode *inode)
17991798
{
1800-
int ret;
1799+
int ret, no_expand;
18011800

1802-
down_write(&EXT4_I(inode)->xattr_sem);
1801+
ext4_write_lock_xattr(inode, &no_expand);
18031802
ret = ext4_destroy_inline_data_nolock(handle, inode);
1804-
up_write(&EXT4_I(inode)->xattr_sem);
1803+
ext4_write_unlock_xattr(inode, &no_expand);
18051804

18061805
return ret;
18071806
}
@@ -1879,7 +1878,7 @@ int ext4_try_to_evict_inline_data(handle_t *handle,
18791878
void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
18801879
{
18811880
handle_t *handle;
1882-
int inline_size, value_len, needed_blocks;
1881+
int inline_size, value_len, needed_blocks, no_expand;
18831882
size_t i_size;
18841883
void *value = NULL;
18851884
struct ext4_xattr_ibody_find is = {
@@ -1896,7 +1895,7 @@ void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
18961895
if (IS_ERR(handle))
18971896
return;
18981897

1899-
down_write(&EXT4_I(inode)->xattr_sem);
1898+
ext4_write_lock_xattr(inode, &no_expand);
19001899
if (!ext4_has_inline_data(inode)) {
19011900
*has_inline = 0;
19021901
ext4_journal_stop(handle);
@@ -1954,7 +1953,7 @@ void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
19541953
up_write(&EXT4_I(inode)->i_data_sem);
19551954
out:
19561955
brelse(is.iloc.bh);
1957-
up_write(&EXT4_I(inode)->xattr_sem);
1956+
ext4_write_unlock_xattr(inode, &no_expand);
19581957
kfree(value);
19591958
if (inode->i_nlink)
19601959
ext4_orphan_del(handle, inode);
@@ -1970,7 +1969,7 @@ void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
19701969

19711970
int ext4_convert_inline_data(struct inode *inode)
19721971
{
1973-
int error, needed_blocks;
1972+
int error, needed_blocks, no_expand;
19741973
handle_t *handle;
19751974
struct ext4_iloc iloc;
19761975

@@ -1992,15 +1991,10 @@ int ext4_convert_inline_data(struct inode *inode)
19921991
goto out_free;
19931992
}
19941993

1995-
down_write(&EXT4_I(inode)->xattr_sem);
1996-
if (!ext4_has_inline_data(inode)) {
1997-
up_write(&EXT4_I(inode)->xattr_sem);
1998-
goto out;
1999-
}
2000-
2001-
error = ext4_convert_inline_data_nolock(handle, inode, &iloc);
2002-
up_write(&EXT4_I(inode)->xattr_sem);
2003-
out:
1994+
ext4_write_lock_xattr(inode, &no_expand);
1995+
if (ext4_has_inline_data(inode))
1996+
error = ext4_convert_inline_data_nolock(handle, inode, &iloc);
1997+
ext4_write_unlock_xattr(inode, &no_expand);
20041998
ext4_journal_stop(handle);
20051999
out_free:
20062000
brelse(iloc.bh);

‎fs/ext4/xattr.c‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,16 +1120,14 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
11201120
struct ext4_xattr_block_find bs = {
11211121
.s = { .not_found = -ENODATA, },
11221122
};
1123-
unsigned long no_expand;
1123+
int no_expand;
11241124
int error;
11251125

11261126
if (!name)
11271127
return -EINVAL;
11281128
if (strlen(name) > 255)
11291129
return -ERANGE;
1130-
down_write(&EXT4_I(inode)->xattr_sem);
1131-
no_expand = ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND);
1132-
ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
1130+
ext4_write_lock_xattr(inode, &no_expand);
11331131

11341132
error = ext4_reserve_inode_write(handle, inode, &is.iloc);
11351133
if (error)
@@ -1190,7 +1188,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
11901188
ext4_xattr_update_super_block(handle, inode->i_sb);
11911189
inode->i_ctime = ext4_current_time(inode);
11921190
if (!value)
1193-
ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
1191+
no_expand = 0;
11941192
error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
11951193
/*
11961194
* The bh is consumed by ext4_mark_iloc_dirty, even with
@@ -1204,9 +1202,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
12041202
cleanup:
12051203
brelse(is.iloc.bh);
12061204
brelse(bs.bh);
1207-
if (no_expand == 0)
1208-
ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
1209-
up_write(&EXT4_I(inode)->xattr_sem);
1205+
ext4_write_unlock_xattr(inode, &no_expand);
12101206
return error;
12111207
}
12121208

@@ -1289,12 +1285,11 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
12891285
void *base, *start, *end;
12901286
int extra_isize = 0, error = 0, tried_min_extra_isize = 0;
12911287
int s_min_extra_isize = le16_to_cpu(EXT4_SB(inode->i_sb)->s_es->s_min_extra_isize);
1288+
int no_expand;
1289+
1290+
if (ext4_write_trylock_xattr(inode, &no_expand) == 0)
1291+
return 0;
12921292

1293-
down_write(&EXT4_I(inode)->xattr_sem);
1294-
/*
1295-
* Set EXT4_STATE_NO_EXPAND to avoid recursion when marking inode dirty
1296-
*/
1297-
ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
12981293
retry:
12991294
if (EXT4_I(inode)->i_extra_isize >= new_extra_isize)
13001295
goto out;
@@ -1487,8 +1482,7 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
14871482
}
14881483
brelse(bh);
14891484
out:
1490-
ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
1491-
up_write(&EXT4_I(inode)->xattr_sem);
1485+
ext4_write_unlock_xattr(inode, &no_expand);
14921486
return 0;
14931487

14941488
cleanup:
@@ -1500,10 +1494,10 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
15001494
kfree(bs);
15011495
brelse(bh);
15021496
/*
1503-
* We deliberately leave EXT4_STATE_NO_EXPAND set here since inode
1504-
* size expansion failed.
1497+
* Inode size expansion failed; don't try again
15051498
*/
1506-
up_write(&EXT4_I(inode)->xattr_sem);
1499+
no_expand = 1;
1500+
ext4_write_unlock_xattr(inode, &no_expand);
15071501
return error;
15081502
}
15091503

‎fs/ext4/xattr.h‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,38 @@ extern const struct xattr_handler ext4_xattr_user_handler;
9898
extern const struct xattr_handler ext4_xattr_trusted_handler;
9999
extern const struct xattr_handler ext4_xattr_security_handler;
100100

101+
/*
102+
* The EXT4_STATE_NO_EXPAND is overloaded and used for two purposes.
103+
* The first is to signal that there the inline xattrs and data are
104+
* taking up so much space that we might as well not keep trying to
105+
* expand it. The second is that xattr_sem is taken for writing, so
106+
* we shouldn't try to recurse into the inode expansion. For this
107+
* second case, we need to make sure that we take save and restore the
108+
* NO_EXPAND state flag appropriately.
109+
*/
110+
static inline void ext4_write_lock_xattr(struct inode *inode, int *save)
111+
{
112+
down_write(&EXT4_I(inode)->xattr_sem);
113+
*save = ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND);
114+
ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
115+
}
116+
117+
static inline int ext4_write_trylock_xattr(struct inode *inode, int *save)
118+
{
119+
if (down_write_trylock(&EXT4_I(inode)->xattr_sem) == 0)
120+
return 0;
121+
*save = ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND);
122+
ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
123+
return 1;
124+
}
125+
126+
static inline void ext4_write_unlock_xattr(struct inode *inode, int *save)
127+
{
128+
if (*save == 0)
129+
ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
130+
up_write(&EXT4_I(inode)->xattr_sem);
131+
}
132+
101133
extern ssize_t ext4_listxattr(struct dentry *, char *, size_t);
102134

103135
extern int ext4_xattr_get(struct inode *, int, const char *, void *, size_t);

0 commit comments

Comments
 (0)