PROBLEM: Corrupting d_count of target dentry, when rename directory to the directory which is mount point, or when rename directory which is mount point to the existing directory.

PROBLEM: Corrupting d_count of target dentry, when rename directory to the directory which is mount point, or when rename directory which is mount point to the existing directory.

[Dmitry Dmitriev]

[-- Attachment #1: Type: text/plain, Size: 4460 bytes --]

Hello!

When rename directory to the directory which is mount point( or rename directory which is mount point to the existing directory ), the target directory dentry d_count become corrupted. This problem occur on 2.6.35 kernel. In this case next call to the dget function report BUG( see below ).

The problem in vfs_rename_dir function( fs/namei.c module ):
2577        target = new_dentry->d_inode;
2578        if (target)
2579                mutex_lock(&target->i_mutex);
2580        if (d_mountpoint(old_dentry)||d_mountpoint(new_dentry))
2581                error = -EBUSY;
2582        else {
2583                if (target)
2584                        dentry_unhash(new_dentry);
2585                error = old_dir->i_op->rename(old_dir, old_dentry, new_dir, new_dentry);
2586        }
2587        if (target) {
2588                if (!error) {
2589                        target->i_flags |= S_DEAD;
2590                        dont_mount(new_dentry);
2591                }
2592                mutex_unlock(&target->i_mutex);
2593                if (d_unhashed(new_dentry))
2594                        d_rehash(new_dentry);
2595                dput(new_dentry);
2596        }

When new_dentry exist, i.e. 'target' is not NULL, and a mount point( or old dentry is a mount point ), then function set 'error' variable to -EBUSY( line 2581 ). After that function will call dput function for new_dentry( line 2595 ) without corresponding dget. In this case d_count of dentry become corrupted. In case when old dentry and new dentry are not mount points, dget for new dentry is called in dentry_unhash function.

Therefore if old dentry or new dentry are mount points, then after execution of vfs_rename_dir function, new dentry will have a corrupted d_count and next call to the dget function can cause a BUG on line 338:
 335static inline struct dentry *dget(struct dentry *dentry)
 336{
 337        if (dentry) {
 338                BUG_ON(!atomic_read(&dentry->d_count));
 339                atomic_inc(&dentry->d_count);
 340        }
 341        return dentry;
 342}

Script reproducing this problem on 2.6.35 kernel is attached.
BUG report is below:
[  679.726754] ------------[ cut here ]------------
[  679.726758] kernel BUG at /build/buildd/linux-2.6.35/include/linux/dcache.h:338!
[  679.726760] invalid opcode: 0000 [#1] SMP 
[  679.726763] last sysfs file: /sys/devices/virtual/block/loop1/queue/hw_sector_size
[  679.726764] Modules linked in: nls_iso8859_1 vfat fat nls_cp437 cifs sha1_generic arc4 ppp_mppe ppp_async crc_ccitt binfmt_misc kvm_intel kvm parport_pc ppdev nfs lockd fscache nfs_acl auth_rpcgss sunrpc i915 drm_kms_helper drm snd_hda_codec_intelhdmi snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer snd_seq_device joydev asus_atk0110 snd intel_agp psmouse soundcore i2c_algo_bit snd_page_alloc serio_raw video output agpgart lp parport hid_a4tech usbhid hid r8169 ahci libahci mii
[  679.726793] 
[  679.726795] Pid: 2463, comm: rmdir Not tainted 2.6.35-23-generic #41-Ubuntu P7H55D-M PRO/System Product Name
[  679.726798] EIP: 0060:[<c0221d8d>] EFLAGS: 00210246 CPU: 1
[  679.726803] EIP is at dentry_unhash+0x7d/0x90
[  679.726805] EAX: 00000000 EBX: ee540bb0 ECX: c02a43e0 EDX: 00000000
[  679.726806] ESI: f2f9e0a0 EDI: ee540bb0 EBP: ef10df1c ESP: ef10df14
[  679.726808]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  679.726810] Process rmdir (pid: 2463, ti=ef10c000 task=f037d8d0 task.ti=ef10c000)
[  679.726811] Stack:
[  679.726812]  fffffff0 f2f9e0a0 ef10df30 c0223126 ef10df3c ee540bb0 00000000 ef10dfa4
[  679.726816] <0> c0224d2d ef10df94 f6bcde00 f2f4ed48 00271d65 00000003 f0187002 00000000
[  679.726820] <0> 00000004 00000000 00000000 00000000 00800004 00000071 00000001 f2874d80
[  679.726824] Call Trace:
[  679.726828]  [<c0223126>] ? vfs_rmdir+0x66/0xe0
[  679.726831]  [<c0224d2d>] ? do_rmdir+0xdd/0xf0
[  679.726835]  [<c0216d1c>] ? filp_close+0x4c/0x80
[  679.726837]  [<c0224d95>] ? sys_rmdir+0x15/0x20
[  679.726841]  [<c05c99f4>] ? syscall_call+0x7/0xb
[  679.726842] Code: 8d b6 00 00 00 00 8b 43 04 a8 10 75 dc 83 c8 10 8b 53 18 89 43 04 8b 43 14 85 c0 89 02 74 03 89 50 04 c7 43 18 00 02 20 00 eb be <0f> 0b eb fe eb 0d 90 90 90 90 90 90 90 90 90 90 90 90 90 55 89 
[  679.726864] EIP: [<c0221d8d>] dentry_unhash+0x7d/0x90 SS:ESP 0068:ef10df14
[  679.726868] ---[ end trace a1a44cd5949fd802 ]---


Regards,
Dmitry

[-- Attachment #2: bug.sh --]
[-- Type: application/octet-stream, Size: 397 bytes --]

#!/bin/sh

IMAGE_FILE=./image_file
MNT_DIR=./mnt
OLD_DIR=./old

dd if=/dev/zero of=$IMAGE_FILE bs=4096 count=4096 2> /dev/null
losetup /dev/loop1 $IMAGE_FILE
mkdosfs -F 32 /dev/loop1 > /dev/null

mkdir $MNT_DIR
mount /dev/loop1 $MNT_DIR

mkdir $OLD_DIR

# rename $OLD_DIR to the $MNT_DIR when $MNT_DIR is a mount point
mv $OLD_DIR $MNT_DIR -T

umount $MNT_DIR

# now will got BUG
rmdir $MNT_DIR

Re: PROBLEM: Corrupting d_count of target dentry, when rename directory to the directory which is mount point, or when rename directory which is mount point to the existing directory.

[Al Viro]

> When new_dentry exist, i.e. 'target' is not NULL, and a mount point( or old dentry is a mount point ), then function set 'error' variable to -EBUSY( line 2581 ). After that function will call dput function for new_dentry( line 2595 ) without corresponding dget. In this case d_count of dentry become corrupted. In case when old dentry and new dentry are not mount points, dget for new dentry is called in dentry_unhash function.

What do you mean, without corresponding dget()?  It's done in dentry_unhash().

Oh, hell...  We don't call it in that case.  Right.  vfs_rmdir() used to
do dentry_unhash() before the checks for mountpoints, vfs_rename_other() -
only after.

The funny part is, it got accidentally fixed in this merge window.  -stable
still needs fixing - by moving dentry_unhash() down past the checks for
d_mountpoint().  Mainline should be OK now...