Messages in this thread Patch in this message | | | Date | Thu, 17 Sep 1998 12:49:20 -0400 | From | Bill Hawes <> | Subject | patch for 2.1.122 msdos-fs |
| |
The attached patch appears to fix a serious problem in the msdos-fs when a directory is moved to another directory. Under these conditions the inodes are switched, with the disk resources moved to the new inode, but the fields in the old inode weren't being cleared. The result was disk corruption of some sort, sometimes with cyclical directory structures.
This is the example script I was using to track down the problem:
#!/bin/bash mkdir -p dir1 mkdir -p dir2 mkdir dir1/tst1 mv -f dir1/tst1 dir2/tst2 mkdir dir1/tst1 mv -f dir2/tst2 dir1/tst1 #disk is now corrupted ... rm -rf dir1/tst1 rm -rf dir2
The change that appears to fix the problem is to clear the old inode i_start and i_logstart fields, and then set i_nlink to 0. I'm not familiar enough with FAT fs to be sure this the fight fix, but it seems to work OK.
The other changes in the patch are minor: a fix for a spurious ENOENT in msdos_lookup, changes in msdos_mkdir to allow cleanup if an error occurs, and a reordering of the test for empty and busy dentries in rmdir.
I've also weakened the test for busy dentries in cross-directory moves to allow a busy target file, as the inode i_busy flag allows in-use files to be deleted.
Regards, Bill--- linux-2.1.122/fs/msdos/namei.c.old Wed Sep 16 21:59:38 1998 +++ linux-2.1.122/fs/msdos/namei.c Thu Sep 17 12:19:16 1998 @@ -266,6 +266,7 @@ res = msdos_find(dir, dentry->d_name.name, dentry->d_name.len, &bh, &de, &ino); + if (res == -ENOENT) goto add; if (res < 0) @@ -275,7 +276,7 @@ /* try to get the inode */ res = -EACCES; - inode = iget(dir->i_sb, ino); + inode = iget(sb, ino); if (!inode) goto out; if (!inode->i_sb || @@ -289,9 +290,9 @@ iput(inode); goto out; } - res = 0; add: d_add(dentry, inode); + res = 0; out: return res; } @@ -446,14 +447,14 @@ if (dir->i_dev != inode->i_dev || dir == inode) printk("msdos_rmdir: impossible condition\n"); /* - * Prune any child dentries, then verify that - * the directory is empty and not in use. + * Check whether the directory is empty, then prune + * any child dentries and make sure it's not in use. */ - shrink_dcache_parent(dentry); res = msdos_empty(inode); if (res) goto rmdir_done; res = -EBUSY; + shrink_dcache_parent(dentry); if (dentry->d_count > 1) { #ifdef MSDOS_DEBUG printk("msdos_rmdir: %s/%s busy, d_count=%d\n", @@ -476,6 +477,7 @@ de->name[0] = DELETED_FLAG; fat_mark_buffer_dirty(sb, bh, 1); res = 0; + rmdir_done: fat_brelse(sb, bh); return res; @@ -499,18 +501,21 @@ return res; is_hid = (dentry->d_name.name[0]=='.') && (msdos_name[0]!='.'); fat_lock_creation(); - if (fat_scan(dir,msdos_name,&bh,&de,&ino,SCAN_ANY) >= 0) { - fat_unlock_creation(); - /* N.B. does this need to be released on the other path? */ - fat_brelse(sb, bh); - return -EEXIST; - } + if (fat_scan(dir,msdos_name,&bh,&de,&ino,SCAN_ANY) >= 0) + goto out_exist; + res = msdos_create_entry(dir,msdos_name,1,is_hid, &inode); if (res < 0) goto out_unlock; + dir->i_nlink++; inode->i_nlink = 2; /* no need to mark them dirty */ MSDOS_I(inode)->i_busy = 1; /* prevent lookups */ + /* + * Instantiate the dentry now, in case we need to cleanup. + */ + d_instantiate(dentry, inode); + if ((res = fat_add_cluster(inode)) < 0) goto mkdir_error; if ((res = msdos_create_entry(inode,MSDOS_DOT,1,0,&dot)) < 0) @@ -521,9 +526,9 @@ dot->i_nlink = inode->i_nlink; mark_inode_dirty(dot); iput(dot); + if ((res = msdos_create_entry(inode,MSDOS_DOTDOT,1,0,&dot)) < 0) goto mkdir_error; - fat_unlock_creation(); dot->i_size = dir->i_size; MSDOS_I(dot)->i_start = MSDOS_I(dir)->i_start; MSDOS_I(dot)->i_logstart = MSDOS_I(dir)->i_logstart; @@ -531,15 +536,22 @@ mark_inode_dirty(dot); MSDOS_I(inode)->i_busy = 0; iput(dot); - d_instantiate(dentry, inode); - return 0; + res = 0; -mkdir_error: - if (msdos_rmdir(dir,dentry) < 0) - fat_fs_panic(dir->i_sb,"rmdir in mkdir failed"); out_unlock: fat_unlock_creation(); return res; + +mkdir_error: + printk("msdos_mkdir: error=%d, attempting cleanup\n", res); + if (msdos_rmdir(dir,dentry) < 0) + fat_fs_panic(dir->i_sb,"rmdir in mkdir failed"); + goto out_unlock; + +out_exist: + fat_brelse(sb, bh); + res = -EEXIST; + goto out_unlock; } /***** Unlink a file */ @@ -636,10 +648,14 @@ if ((old_de->attr & ATTR_SYS)) goto out_error; + if (S_ISDIR(new_inode->i_mode)) { + /* make sure it's empty */ + error = msdos_empty(new_inode); + if (error) + goto out_error; #ifdef MSDOS_CHECK_BUSY - /* check for a busy dentry */ - error = -EBUSY; - if (new_dentry->d_count > 1) { + /* check for a busy dentry */ + error = -EBUSY; shrink_dcache_parent(new_dentry); if (new_dentry->d_count > 1) { printk("msdos_rename_same: %s/%s busy, count=%d\n", @@ -647,20 +663,19 @@ new_dentry->d_count); goto out_error; } - } #endif - - if (S_ISDIR(new_inode->i_mode)) { new_dir->i_nlink--; mark_inode_dirty(new_dir); } new_inode->i_nlink = 0; MSDOS_I(new_inode)->i_busy = 1; mark_inode_dirty(new_inode); -#ifdef MSDOS_CHECK_BUSY - /* d_delete the dentry, as we killed its inode */ - d_delete(new_dentry); -#endif + /* + * Make it negative if it's not busy; + * otherwise let d_move() drop it. + */ + if (new_dentry->d_count == 1) + d_delete(new_dentry); new_de->name[0] = DELETED_FLAG; fat_mark_buffer_dirty(sb, new_bh, 1); @@ -763,16 +778,22 @@ } #endif if (S_ISDIR(new_inode->i_mode)) { + /* make sure it's empty */ + error = msdos_empty(new_inode); + if (error) + goto out_new; new_dir->i_nlink--; mark_inode_dirty(new_dir); } new_inode->i_nlink = 0; MSDOS_I(new_inode)->i_busy = 1; mark_inode_dirty(new_inode); -#ifdef MSDOS_CHECK_BUSY - /* d_delete the dentry, as we killed its inode */ - d_delete(new_dentry); -#endif + /* + * Make it negative if it's not busy; + * otherwise let d_move() drop it. + */ + if (new_dentry->d_count == 1) + d_delete(new_dentry); new_de->name[0] = DELETED_FLAG; fat_mark_buffer_dirty(sb, new_bh, 1); fat_brelse(sb, new_bh); @@ -804,12 +825,20 @@ free_inode = iget(sb, free_ino); if (!free_inode) goto out_iput; + /* make sure it's not busy! */ + if (MSDOS_I(free_inode)->i_busy) + printk(KERN_ERR "msdos_rename_diff: new inode %ld busy!\n", + (ino_t) free_ino); + if (!list_empty(&free_inode->i_dentry)) + printk("msdos_rename_diff: free inode has aliases??\n"); msdos_read_inode(free_inode); + fat_mark_buffer_dirty(sb, free_bh, 1); /* * Make sure the old dentry isn't busy, * as we need to change inodes ... */ + error = -EBUSY; if (old_dentry->d_count > 1) { shrink_dcache_parent(old_dentry); if (old_dentry->d_count > 1) { @@ -836,6 +865,11 @@ MSDOS_I(free_inode)->i_logstart = MSDOS_I(old_inode)->i_logstart; MSDOS_I(free_inode)->i_attrs = MSDOS_I(old_inode)->i_attrs; + /* release the old inode's resources */ + MSDOS_I(old_inode)->i_start = 0; + MSDOS_I(old_inode)->i_logstart = 0; + old_inode->i_nlink = 0; + /* * Install the new inode ... */ @@ -845,7 +879,6 @@ mark_inode_dirty(old_inode); old_de->name[0] = DELETED_FLAG; fat_mark_buffer_dirty(sb, old_bh, 1); - fat_mark_buffer_dirty(sb, free_bh, 1); iput(old_inode); /* a directory? */ @@ -854,13 +887,13 @@ MSDOS_I(dotdot_inode)->i_logstart = MSDOS_I(new_dir)->i_logstart; dotdot_de->start = CT_LE_W(MSDOS_I(new_dir)->i_logstart); dotdot_de->starthi = CT_LE_W((MSDOS_I(new_dir)->i_logstart) >> 16); - mark_inode_dirty(dotdot_inode); - fat_mark_buffer_dirty(sb, dotdot_bh, 1); old_dir->i_nlink--; new_dir->i_nlink++; /* no need to mark them dirty */ dotdot_inode->i_nlink = new_dir->i_nlink; + mark_inode_dirty(dotdot_inode); iput(dotdot_inode); + fat_mark_buffer_dirty(sb, dotdot_bh, 1); fat_brelse(sb, dotdot_bh); } | |