Skip to content

Commit c755e25

Browse files
tytsotytso
committed
ext4: fix deadlock between inline_data and ext4_expand_extra_isize_ea()
The xattr_sem deadlock problems fixed in commit 2e81a4e: "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 2e81a4e. 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 torvalds#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] #2: (jbd2_handle){++++..}, at: [<c123314a>] start_this_handle+0x582/0x622 [ 11.161960] #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 torvalds#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: 08168ac 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 2e81a4e as a prereq) Reported-by: George Spelvin <linux@sciencehorizons.net> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
1 parent 670e987 commit c755e25

File tree

3 files changed

+74
-54
lines changed

3 files changed

+74
-54
lines changed

‎fs/ext4/inline.c‎

Lines changed: 30 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ static int ext4_update_inline_data(handle_t *handle, struct inode *inode,
381381
static int ext4_prepare_inline_data(handle_t *handle, struct inode *inode,
382382
unsigned int len)
383383
{
384-
int ret, size;
384+
int ret, size, no_expand;
385385
struct ext4_inode_info *ei = EXT4_I(inode);
386386

387387
if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
@@ -391,15 +391,14 @@ static int ext4_prepare_inline_data(handle_t *handle, struct inode *inode,
391391
if (size < len)
392392
return -ENOSPC;
393393

394-
down_write(&EXT4_I(inode)->xattr_sem);
394+
ext4_write_lock_xattr(inode, &no_expand);
395395

396396
if (ei->i_inline_off)
397397
ret = ext4_update_inline_data(handle, inode, len);
398398
else
399399
ret = ext4_create_inline_data(handle, inode, len);
400400

401-
up_write(&EXT4_I(inode)->xattr_sem);
402-
401+
ext4_write_unlock_xattr(inode, &no_expand);
403402
return ret;
404403
}
405404

@@ -533,7 +532,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
533532
struct inode *inode,
534533
unsigned flags)
535534
{
536-
int ret, needed_blocks;
535+
int ret, needed_blocks, no_expand;
537536
handle_t *handle = NULL;
538537
int retries = 0, sem_held = 0;
539538
struct page *page = NULL;
@@ -573,7 +572,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
573572
goto out;
574573
}
575574

576-
down_write(&EXT4_I(inode)->xattr_sem);
575+
ext4_write_lock_xattr(inode, &no_expand);
577576
sem_held = 1;
578577
/* If some one has already done this for us, just exit. */
579578
if (!ext4_has_inline_data(inode)) {
@@ -610,7 +609,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
610609
put_page(page);
611610
page = NULL;
612611
ext4_orphan_add(handle, inode);
613-
up_write(&EXT4_I(inode)->xattr_sem);
612+
ext4_write_unlock_xattr(inode, &no_expand);
614613
sem_held = 0;
615614
ext4_journal_stop(handle);
616615
handle = NULL;
@@ -636,7 +635,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
636635
put_page(page);
637636
}
638637
if (sem_held)
639-
up_write(&EXT4_I(inode)->xattr_sem);
638+
ext4_write_unlock_xattr(inode, &no_expand);
640639
if (handle)
641640
ext4_journal_stop(handle);
642641
brelse(iloc.bh);
@@ -729,7 +728,7 @@ int ext4_try_to_write_inline_data(struct address_space *mapping,
729728
int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
730729
unsigned copied, struct page *page)
731730
{
732-
int ret;
731+
int ret, no_expand;
733732
void *kaddr;
734733
struct ext4_iloc iloc;
735734

@@ -747,7 +746,7 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
747746
goto out;
748747
}
749748

750-
down_write(&EXT4_I(inode)->xattr_sem);
749+
ext4_write_lock_xattr(inode, &no_expand);
751750
BUG_ON(!ext4_has_inline_data(inode));
752751

753752
kaddr = kmap_atomic(page);
@@ -757,7 +756,7 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
757756
/* clear page dirty so that writepages wouldn't work for us. */
758757
ClearPageDirty(page);
759758

760-
up_write(&EXT4_I(inode)->xattr_sem);
759+
ext4_write_unlock_xattr(inode, &no_expand);
761760
brelse(iloc.bh);
762761
out:
763762
return copied;
@@ -768,7 +767,7 @@ ext4_journalled_write_inline_data(struct inode *inode,
768767
unsigned len,
769768
struct page *page)
770769
{
771-
int ret;
770+
int ret, no_expand;
772771
void *kaddr;
773772
struct ext4_iloc iloc;
774773

@@ -778,11 +777,11 @@ ext4_journalled_write_inline_data(struct inode *inode,
778777
return NULL;
779778
}
780779

781-
down_write(&EXT4_I(inode)->xattr_sem);
780+
ext4_write_lock_xattr(inode, &no_expand);
782781
kaddr = kmap_atomic(page);
783782
ext4_write_inline_data(inode, &iloc, kaddr, 0, len);
784783
kunmap_atomic(kaddr);
785-
up_write(&EXT4_I(inode)->xattr_sem);
784+
ext4_write_unlock_xattr(inode, &no_expand);
786785

787786
return iloc.bh;
788787
}
@@ -1259,15 +1258,15 @@ static int ext4_convert_inline_data_nolock(handle_t *handle,
12591258
int ext4_try_add_inline_entry(handle_t *handle, struct ext4_filename *fname,
12601259
struct inode *dir, struct inode *inode)
12611260
{
1262-
int ret, inline_size;
1261+
int ret, inline_size, no_expand;
12631262
void *inline_start;
12641263
struct ext4_iloc iloc;
12651264

12661265
ret = ext4_get_inode_loc(dir, &iloc);
12671266
if (ret)
12681267
return ret;
12691268

1270-
down_write(&EXT4_I(dir)->xattr_sem);
1269+
ext4_write_lock_xattr(dir, &no_expand);
12711270
if (!ext4_has_inline_data(dir))
12721271
goto out;
12731272

@@ -1313,7 +1312,7 @@ int ext4_try_add_inline_entry(handle_t *handle, struct ext4_filename *fname,
13131312

13141313
out:
13151314
ext4_mark_inode_dirty(handle, dir);
1316-
up_write(&EXT4_I(dir)->xattr_sem);
1315+
ext4_write_unlock_xattr(dir, &no_expand);
13171316
brelse(iloc.bh);
13181317
return ret;
13191318
}
@@ -1673,15 +1672,15 @@ int ext4_delete_inline_entry(handle_t *handle,
16731672
struct buffer_head *bh,
16741673
int *has_inline_data)
16751674
{
1676-
int err, inline_size;
1675+
int err, inline_size, no_expand;
16771676
struct ext4_iloc iloc;
16781677
void *inline_start;
16791678

16801679
err = ext4_get_inode_loc(dir, &iloc);
16811680
if (err)
16821681
return err;
16831682

1684-
down_write(&EXT4_I(dir)->xattr_sem);
1683+
ext4_write_lock_xattr(dir, &no_expand);
16851684
if (!ext4_has_inline_data(dir)) {
16861685
*has_inline_data = 0;
16871686
goto out;
@@ -1715,7 +1714,7 @@ int ext4_delete_inline_entry(handle_t *handle,
17151714

17161715
ext4_show_inline_dir(dir, iloc.bh, inline_start, inline_size);
17171716
out:
1718-
up_write(&EXT4_I(dir)->xattr_sem);
1717+
ext4_write_unlock_xattr(dir, &no_expand);
17191718
brelse(iloc.bh);
17201719
if (err != -ENOENT)
17211720
ext4_std_error(dir->i_sb, err);
@@ -1814,11 +1813,11 @@ bool empty_inline_dir(struct inode *dir, int *has_inline_data)
18141813

18151814
int ext4_destroy_inline_data(handle_t *handle, struct inode *inode)
18161815
{
1817-
int ret;
1816+
int ret, no_expand;
18181817

1819-
down_write(&EXT4_I(inode)->xattr_sem);
1818+
ext4_write_lock_xattr(inode, &no_expand);
18201819
ret = ext4_destroy_inline_data_nolock(handle, inode);
1821-
up_write(&EXT4_I(inode)->xattr_sem);
1820+
ext4_write_unlock_xattr(inode, &no_expand);
18221821

18231822
return ret;
18241823
}
@@ -1903,7 +1902,7 @@ int ext4_try_to_evict_inline_data(handle_t *handle,
19031902
void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
19041903
{
19051904
handle_t *handle;
1906-
int inline_size, value_len, needed_blocks;
1905+
int inline_size, value_len, needed_blocks, no_expand;
19071906
size_t i_size;
19081907
void *value = NULL;
19091908
struct ext4_xattr_ibody_find is = {
@@ -1920,7 +1919,7 @@ void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
19201919
if (IS_ERR(handle))
19211920
return;
19221921

1923-
down_write(&EXT4_I(inode)->xattr_sem);
1922+
ext4_write_lock_xattr(inode, &no_expand);
19241923
if (!ext4_has_inline_data(inode)) {
19251924
*has_inline = 0;
19261925
ext4_journal_stop(handle);
@@ -1978,7 +1977,7 @@ void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
19781977
up_write(&EXT4_I(inode)->i_data_sem);
19791978
out:
19801979
brelse(is.iloc.bh);
1981-
up_write(&EXT4_I(inode)->xattr_sem);
1980+
ext4_write_unlock_xattr(inode, &no_expand);
19821981
kfree(value);
19831982
if (inode->i_nlink)
19841983
ext4_orphan_del(handle, inode);
@@ -1994,7 +1993,7 @@ void ext4_inline_data_truncate(struct inode *inode, int *has_inline)
19941993

19951994
int ext4_convert_inline_data(struct inode *inode)
19961995
{
1997-
int error, needed_blocks;
1996+
int error, needed_blocks, no_expand;
19981997
handle_t *handle;
19991998
struct ext4_iloc iloc;
20001999

@@ -2016,15 +2015,10 @@ int ext4_convert_inline_data(struct inode *inode)
20162015
goto out_free;
20172016
}
20182017

2019-
down_write(&EXT4_I(inode)->xattr_sem);
2020-
if (!ext4_has_inline_data(inode)) {
2021-
up_write(&EXT4_I(inode)->xattr_sem);
2022-
goto out;
2023-
}
2024-
2025-
error = ext4_convert_inline_data_nolock(handle, inode, &iloc);
2026-
up_write(&EXT4_I(inode)->xattr_sem);
2027-
out:
2018+
ext4_write_lock_xattr(inode, &no_expand);
2019+
if (ext4_has_inline_data(inode))
2020+
error = ext4_convert_inline_data_nolock(handle, inode, &iloc);
2021+
ext4_write_unlock_xattr(inode, &no_expand);
20282022
ext4_journal_stop(handle);
20292023
out_free:
20302024
brelse(iloc.bh);

‎fs/ext4/xattr.c‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,16 +1188,14 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
11881188
struct ext4_xattr_block_find bs = {
11891189
.s = { .not_found = -ENODATA, },
11901190
};
1191-
unsigned long no_expand;
1191+
int no_expand;
11921192
int error;
11931193

11941194
if (!name)
11951195
return -EINVAL;
11961196
if (strlen(name) > 255)
11971197
return -ERANGE;
1198-
down_write(&EXT4_I(inode)->xattr_sem);
1199-
no_expand = ext4_test_inode_state(inode, EXT4_STATE_NO_EXPAND);
1200-
ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
1198+
ext4_write_lock_xattr(inode, &no_expand);
12011199

12021200
error = ext4_reserve_inode_write(handle, inode, &is.iloc);
12031201
if (error)
@@ -1264,7 +1262,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
12641262
ext4_xattr_update_super_block(handle, inode->i_sb);
12651263
inode->i_ctime = current_time(inode);
12661264
if (!value)
1267-
ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
1265+
no_expand = 0;
12681266
error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
12691267
/*
12701268
* The bh is consumed by ext4_mark_iloc_dirty, even with
@@ -1278,9 +1276,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
12781276
cleanup:
12791277
brelse(is.iloc.bh);
12801278
brelse(bs.bh);
1281-
if (no_expand == 0)
1282-
ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
1283-
up_write(&EXT4_I(inode)->xattr_sem);
1279+
ext4_write_unlock_xattr(inode, &no_expand);
12841280
return error;
12851281
}
12861282

@@ -1497,12 +1493,11 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
14971493
int error = 0, tried_min_extra_isize = 0;
14981494
int s_min_extra_isize = le16_to_cpu(EXT4_SB(inode->i_sb)->s_es->s_min_extra_isize);
14991495
int isize_diff; /* How much do we need to grow i_extra_isize */
1496+
int no_expand;
1497+
1498+
if (ext4_write_trylock_xattr(inode, &no_expand) == 0)
1499+
return 0;
15001500

1501-
down_write(&EXT4_I(inode)->xattr_sem);
1502-
/*
1503-
* Set EXT4_STATE_NO_EXPAND to avoid recursion when marking inode dirty
1504-
*/
1505-
ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
15061501
retry:
15071502
isize_diff = new_extra_isize - EXT4_I(inode)->i_extra_isize;
15081503
if (EXT4_I(inode)->i_extra_isize >= new_extra_isize)
@@ -1584,17 +1579,16 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
15841579
EXT4_I(inode)->i_extra_isize = new_extra_isize;
15851580
brelse(bh);
15861581
out:
1587-
ext4_clear_inode_state(inode, EXT4_STATE_NO_EXPAND);
1588-
up_write(&EXT4_I(inode)->xattr_sem);
1582+
ext4_write_unlock_xattr(inode, &no_expand);
15891583
return 0;
15901584

15911585
cleanup:
15921586
brelse(bh);
15931587
/*
1594-
* We deliberately leave EXT4_STATE_NO_EXPAND set here since inode
1595-
* size expansion failed.
1588+
* Inode size expansion failed; don't try again
15961589
*/
1597-
up_write(&EXT4_I(inode)->xattr_sem);
1590+
no_expand = 1;
1591+
ext4_write_unlock_xattr(inode, &no_expand);
15981592
return error;
15991593
}
16001594

‎fs/ext4/xattr.h‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,38 @@ extern const struct xattr_handler ext4_xattr_security_handler;
102102

103103
#define EXT4_XATTR_NAME_ENCRYPTION_CONTEXT "c"
104104

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

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

0 commit comments

Comments
 (0)