[LU-14710] check_dot() does not handle dirdata/FID entry properly Created: 26/May/21 Updated: 14/Nov/22 |
|
| Status: | Open |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Minor |
| Reporter: | Andreas Dilger | Assignee: | WC Triage |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Issue Links: |
|
||||||||||||
| Severity: | 3 | ||||||||||||
| Rank (Obsolete): | 9223372036854775807 | ||||||||||||
| Description |
|
In check_dot() it assumes that the "." and ".." entries are fixed at 12-bytes, but with the dirdata feature storing a FID in these directory entries, they will be larger. If there is some unrelated problem in the directory returned by e2fsck_check_dirent_data() then this check will incorrectly cause the "." and ".." entries to be recreated and clobber the FIDs. static int check_dot(...) { : dir_data_error = e2fsck_check_dirent_data(ctx, dirent, offset, pctx); : if (rec_len > 12) { new_len = rec_len - 12; if (new_len > 12 && dir_data_error) { if (created || fix_problem(ctx, PR_2_SPLIT_DOT, pctx)) { nextdir = (struct ext2_dir_entry *) ((char *) dirent + 12); dirent->rec_len = 12; (void) ext2fs_set_rec_len(ctx->fs, new_len, nextdir); nextdir->inode = 0; ext2fs_dirent_set_name_len(nextdir, 0); ext2fs_dirent_set_file_type(nextdir, EXT2_FT_UNKNOWN); As a follow-on, the HTree index will no longer immediately follow the ".." entry (fake_root) and be invalidated: Directory entry for '.' in ... (1032783) is big. Split? no Second entry '3.3.0' (inode=538027 fid=[0x380020941:0x4c38:0x0]) in directory inode 1032783 should be '..' Fix? no The code in check_dot() needs to be updated to properly check for the length of dirdata entries. It doesn't look like same bug appears in check_dotdot(). It is checking for (rec_len < 12), which should be OK, since dirdata entries will always be larger. Properly accounting for rec_len in check_dot() should avoid any problems in check_dotdot(). However, there is LU-7399 which suggests an improvement for rebuilding ".." in other situations. |
| Comments |
| Comment by Andreas Dilger [ 27/May/21 ] |
|
In further investigation, it appears that check_dot() does do the right thing, in that the "." entry is created with rec_len=4096, which "hides" the ".." entry at offset 12 in the directory block. This appears to be a bug introduced by osd-ldiskfs, TBD. check_dot() goes on to repair the "." dirent to have rec_len=12, and this would allow the existing ".." entry to be recovered, but it is instead clobbered. It would make sense to have check_dot() do a quick sanity check and "recover" the ".." entry if it looks reasonable (name="..", namelen=2, and rec_len < (blocksize-12), and then leave it up to check_dotdot() to do the full sanity check, instead of unconditionally zeroing out the next entry. |
| Comment by Gerrit Updater [ 28/May/21 ] |
|
Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/43858 |
| Comment by Gerrit Updater [ 31/May/21 ] |
|
Andreas Dilger (adilger@whamcloud.com) merged in patch https://review.whamcloud.com/43858/ |
| Comment by Andreas Dilger [ 14/Nov/22 ] |
|
With dirdata enabled, the ".." entry may not be at offset 12, so it would be more graceful to do a quick scan to see if ".." is at offset "12+20=32" (1 byte NUL + 1 byte size + 16 byte lu_fid + alignment). That wouldn't "preserve" the dirdata for "." but it might keep it for "..", which is more important anyway (for fid2path). |