[LU-14710] check_dot() does not handle dirdata/FID entry properly

• Type: Bug
• Resolution: Unresolved
• Priority: Minor
• Severity: 3
• Assignee: WC Triage
• Reporter: Andreas Dilger

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.

Issue Links

• is related to: LU-14719 "lfs migrate -m" creates broken agent inodes when target MDT full
• is related to: LU-7399 e2fsck repair of ".." entries from LU-5626 should be improved

Comments

[Andreas Dilger]

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).

[Gerrit Updater]

Andreas Dilger ([e-mail]) merged in patch https://review.whamcloud.com/43858/
Subject: LU-14710 e2fsck: fix ".." more gracefully if possible
Project: tools/e2fsprogs
Branch: master-lustre
Current Patch Set:
Commit: 5aa781e24b9ce22049c8542daaf47a05027a1179

[Gerrit Updater]

Andreas Dilger ([e-mail]) uploaded a new patch: https://review.whamcloud.com/43858
Subject: LU-14710 e2fsck: fix ".." more gracefully if possible
Project: tools/e2fsprogs
Branch: master-lustre
Current Patch Set: 1
Commit: a5ace419651937b0f8c10b5cec5a8015f6780d28

[Andreas Dilger]

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.