ni_insert_nonresident() can create an attribute list and move the base $DATA attribute to another MFT record, which invalidates attr_b and mi_b. attr_allocate_frame() kept using the stale pointers afterwards to update valid_size, writing into a record that no longer holds the attribute. The same situation is already handled in attr_data_get_block_locked(), which re-reads the base attribute after the layout changes. Look up attr_b again after the insertion, and read valid_size from the attribute before the layout can change. While here, flatten the 'if (attr)' block in attr_allocate_frame() by returning early to ins_ext, and simplify an empty-bodied if in attr_data_get_block_locked(). Signed-off-by: Konstantin Komarov --- fs/ntfs3/attrib.c | 109 ++++++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 52 deletions(-) diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c index 928c91b57da5..922134cabb05 100644 --- a/fs/ntfs3/attrib.c +++ b/fs/ntfs3/attrib.c @@ -1019,11 +1019,8 @@ int attr_data_get_block_locked(struct ntfs_inode *ni, CLST vcn, CLST clen, int step; again: - if (run_lookup_entry_da(run, da ? &ni->file.run_da : NULL, vcn, lcn, - len)) { - } else { + if (!run_lookup_entry_da(run, da ? run_da : NULL, vcn, lcn, len)) *len = 0; - } if (*len) { if (*lcn != SPARSE_LCN || !new) @@ -1812,7 +1809,7 @@ int attr_allocate_frame(struct ntfs_inode *ni, CLST frame, size_t compr_size, struct ATTR_LIST_ENTRY *le, *le_b; struct mft_inode *mi, *mi_b; CLST svcn, evcn1, next_svcn, len; - CLST vcn, end, clst_data; + CLST vcn, end, clst_data, alloc, evcn; u64 total_size, valid_size, data_size; le_b = NULL; @@ -1830,6 +1827,7 @@ int attr_allocate_frame(struct ntfs_inode *ni, CLST frame, size_t compr_size, svcn = le64_to_cpu(attr_b->nres.svcn); evcn1 = le64_to_cpu(attr_b->nres.evcn) + 1; data_size = le64_to_cpu(attr_b->nres.data_size); + valid_size = le64_to_cpu(attr_b->nres.valid_size); if (svcn <= vcn && vcn < evcn1) { attr = attr_b; @@ -1949,63 +1947,63 @@ int attr_allocate_frame(struct ntfs_inode *ni, CLST frame, size_t compr_size, attr = ni_find_attr(ni, attr, &le, ATTR_DATA, ni->file.ads.name, ni->file.ads.len, &svcn, &mi); - if (attr) { - CLST alloc = bytes_to_cluster( - sbi, le64_to_cpu(attr_b->nres.alloc_size)); - CLST evcn = le64_to_cpu(attr->nres.evcn); - - if (end < next_svcn) - end = next_svcn; - while (end > evcn) { - /* Remove segment [svcn : evcn). */ - mi_remove_attr(NULL, mi, attr); - - if (!al_remove_le(ni, le)) { - err = -EINVAL; - goto out; - } + if (!attr) + goto ins_ext; - if (evcn + 1 >= alloc) { - /* Last attribute segment. */ - evcn1 = evcn + 1; - goto ins_ext; - } + alloc = bytes_to_cluster(sbi, le64_to_cpu(attr_b->nres.alloc_size)); + evcn = le64_to_cpu(attr->nres.evcn); - if (ni_load_mi(ni, le, &mi)) { - attr = NULL; - goto out; - } + if (end < next_svcn) + end = next_svcn; + while (end > evcn) { + /* Remove segment [svcn : evcn). */ + mi_remove_attr(NULL, mi, attr); - attr = mi_find_attr(ni, mi, NULL, ATTR_DATA, - ni->file.ads.name, ni->file.ads.len, - &le->id); - if (!attr) { - err = -EINVAL; - goto out; - } - svcn = le64_to_cpu(attr->nres.svcn); - evcn = le64_to_cpu(attr->nres.evcn); + if (!al_remove_le(ni, le)) { + err = -EINVAL; + goto out; } - if (end < svcn) - end = svcn; + if (evcn + 1 >= alloc) { + /* Last attribute segment. */ + evcn1 = evcn + 1; + goto ins_ext; + } - err = attr_load_runs(attr, ni, run, &end); - if (err) + if (ni_load_mi(ni, le, &mi)) { + attr = NULL; goto out; + } - evcn1 = evcn + 1; - attr->nres.svcn = cpu_to_le64(next_svcn); - err = mi_pack_runs(mi, attr, run, evcn1 - next_svcn); - if (err) + attr = mi_find_attr(ni, mi, NULL, ATTR_DATA, ni->file.ads.name, + ni->file.ads.len, &le->id); + if (!attr) { + err = -EINVAL; goto out; + } + svcn = le64_to_cpu(attr->nres.svcn); + evcn = le64_to_cpu(attr->nres.evcn); + } - le->vcn = cpu_to_le64(next_svcn); - ni->attr_list.dirty = true; - mi->dirty = true; + if (end < svcn) + end = svcn; + + err = attr_load_runs(attr, ni, run, &end); + if (err) + goto out; + + evcn1 = evcn + 1; + attr->nres.svcn = cpu_to_le64(next_svcn); + err = mi_pack_runs(mi, attr, run, evcn1 - next_svcn); + if (err) + goto out; + + le->vcn = cpu_to_le64(next_svcn); + ni->attr_list.dirty = true; + mi->dirty = true; + + next_svcn = le64_to_cpu(attr->nres.evcn) + 1; - next_svcn = le64_to_cpu(attr->nres.evcn) + 1; - } ins_ext: if (evcn1 > next_svcn) { err = ni_insert_nonresident(ni, ATTR_DATA, ni->file.ads.name, @@ -2014,6 +2012,14 @@ int attr_allocate_frame(struct ntfs_inode *ni, CLST frame, size_t compr_size, &attr, &mi, NULL); if (err) goto out; + /* Layout of records is changed. */ + attr_b = ni_find_attr(ni, NULL, NULL, ATTR_DATA, + ni->file.ads.name, ni->file.ads.len, NULL, + &mi_b); + if (!attr_b) { + err = -EINVAL; + goto out; + } } ok: run_truncate_around(run, vcn); @@ -2022,7 +2028,6 @@ int attr_allocate_frame(struct ntfs_inode *ni, CLST frame, size_t compr_size, if (new_valid > data_size) new_valid = data_size; - valid_size = le64_to_cpu(attr_b->nres.valid_size); if (new_valid != valid_size) { attr_b->nres.valid_size = cpu_to_le64(new_valid); mi_b->dirty = true; -- 2.43.0