linux-kernel - Re: [PATCH] Allow d_splice_alias to accept hashed dentries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <4EFEE26A-CAB0-4742-86FF-942FC898E3CE@linuxhacker.ru>
Date:	Thu, 2 Jun 2016 20:25:10 -0400
From:	Oleg Drokin <green@...uxhacker.ru>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	"J. Bruce Fields" <bfields@...hat.com>,
	"<linux-kernel@...r.kernel.org> Mailing List" 
	<linux-kernel@...r.kernel.org>,
	"<linux-fsdevel@...r.kernel.org>" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH] Allow d_splice_alias to accept hashed dentries


On Jun 2, 2016, at 7:59 PM, green@...uxhacker.ru wrote:

> From: Oleg Drokin <green@...uxhacker.ru>
> 
> The comment at the top of the function already permits it,
> just take out the assetrtion and update __d_add to
> be aware of this. Move the BUG_ON to d_add which is the
> other user of __d_add and presumably we do not want to
> allow hashed dentries as input there.
> 
> Signed-off-by: Oleg Drokin <green@...uxhacker.ru>
> ---
> So seeing how there's only one other __d_add user, I just moved the BUG_ON
> to d_add and allowed __d_add to not call _d_rehash if the dentry is
> already hashed.
> This seems to be working as expected and I don't see any problems with it.
> Of course I am probably missing some other important consideration ;)

I guess I was too fast to assume it was fine, or I just hit another bug,
but with the patch below I am now getting:

[ 1120.040403] BUG: unable to handle kernel paging request at ffff880119fc3000
[ 1120.040883] IP: [<ffffffff817f87d4>] bad_gs+0xd1d/0x1ba9
[ 1120.041302] PGD 2dca067 PUD 2dcd067 PMD 11ff31067 PTE 8000000119fc3060
[ 1120.041699] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[ 1120.042019] Modules linked in: loop rpcsec_gss_krb5 joydev pcspkr acpi_cpufreq i2c_piix4 tpm_tis tpm nfsd drm_kms_helper ttm drm serio_raw virtio_blk
[ 1120.042803] CPU: 5 PID: 16997 Comm: file_concat.sh Not tainted 4.7.0-rc1-vm-nfs+ #101
[ 1120.043467] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 1120.043826] task: ffff8800c34f40c0 ti: ffff8800c15b8000 task.ti: ffff8800c15b8000
[ 1120.044446] RIP: 0010:[<ffffffff817f87d4>]  [<ffffffff817f87d4>] bad_gs+0xd1d/0x1ba9
[ 1120.045099] RSP: 0018:ffff8800c15bbba0  EFLAGS: 00010286
[ 1120.045439] RAX: ffff8800d20f0f30 RBX: ffff8800d20f0f00 RCX: ffff880119fc3000
[ 1120.045815] RDX: ffff880119fc3000 RSI: 0000000000000000 RDI: 0000000000000001
[ 1120.046168] RBP: ffff8800c15bbbe8 R08: 0000000000000000 R09: ffff8800d20f0ed0
[ 1120.046526] R10: 0000000000000059 R11: ffff8800c34f4c38 R12: ffff8800d1de9ed0
[ 1120.046892] R13: ffff8800c15bbdf0 R14: ffff8800d20f0f50 R15: 00000000f7a5b27a
[ 1120.047248] FS:  00007f57e93d8700(0000) GS:ffff88011f540000(0000) knlGS:0000000000000000
[ 1120.047890] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1120.048223] CR2: ffff880119fc3000 CR3: 00000000d4166000 CR4: 00000000000006e0
[ 1120.048596] Stack:
[ 1120.048892]  ffffffff81243a35 0000000000000096 ffff880119fc3000 ffffffff00000001
[ 1120.049537]  000000000000c2aa ffff8800c15bbdf0 ffff8800d1de9ed0 ffff8800c15bbde0
[ 1120.050189]  ffff8800c15bbdf0 ffff8800c15bbc18 ffffffff81243c64 ffffffff81236c4e
[ 1120.050846] Call Trace:
[ 1120.051146]  [<ffffffff81243a35>] ? __d_lookup+0x5/0x1b0
[ 1120.051485]  [<ffffffff81243c64>] d_lookup+0x84/0xb0
[ 1120.051828]  [<ffffffff81236c4e>] ? lookup_open+0xfe/0x7a0
[ 1120.052160]  [<ffffffff81236c4e>] lookup_open+0xfe/0x7a0
[ 1120.052500]  [<ffffffff812377ce>] path_openat+0x4de/0xfc0
[ 1120.052846]  [<ffffffff8123935e>] do_filp_open+0x7e/0xe0
[ 1120.053177]  [<ffffffff81233110>] ? lock_rename+0x100/0x100
[ 1120.053524]  [<ffffffff817f4957>] ? _raw_spin_unlock+0x27/0x40
[ 1120.053874]  [<ffffffff8124875c>] ? __alloc_fd+0xbc/0x170
[ 1120.054202]  [<ffffffff81226896>] do_sys_open+0x116/0x1f0
[ 1120.054540]  [<ffffffff8122698e>] SyS_open+0x1e/0x20
[ 1120.054878]  [<ffffffff817f5136>] entry_SYSCALL_64_fastpath+0x1e/0xad
[ 1120.055221] Code: e1 03 49 d3 e8 e9 41 ae a4 ff 48 8d 0a 48 83 e1 f8 4c 8b 01 8d 0a 83 e1 07 c1 e1 03 49 d3 e8 e9 00 af a4 ff 48 8d 0a 48 83 e1 f8 <4c> 8b 01 8d 0a 83 e1 07 c1 e1 03 49 d3 e8 e9 7b b3 a4 ff b9 f2 
[ 1120.056560] RIP  [<ffffffff817f87d4>] bad_gs+0xd1d/0x1ba9
[ 1120.056909]  RSP <ffff8800c15bbba0>
[ 1120.057220] CR2: ffff880119fc3000

> 
> fs/dcache.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index ad4a542..0e35049 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2565,7 +2565,8 @@ static inline void __d_add(struct dentry *dentry, struct inode *inode)
> 		raw_write_seqcount_end(&dentry->d_seq);
> 		__fsnotify_d_instantiate(dentry);
> 	}
> -	_d_rehash(dentry);
> +	if (d_unhashed(dentry))
> +		_d_rehash(dentry);
> 	if (dir)
> 		end_dir_add(dir, n);
> 	spin_unlock(&dentry->d_lock);
> @@ -2584,6 +2585,8 @@ static inline void __d_add(struct dentry *dentry, struct inode *inode)
> 
> void d_add(struct dentry *entry, struct inode *inode)
> {
> +	BUG_ON(!d_unhashed(entry));
> +
> 	if (inode) {
> 		security_d_instantiate(entry, inode);
> 		spin_lock(&inode->i_lock);
> @@ -2986,8 +2989,6 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
> 	if (IS_ERR(inode))
> 		return ERR_CAST(inode);
> 
> -	BUG_ON(!d_unhashed(dentry));
> -
> 	if (!inode)
> 		goto out;
> 
> -- 
> 2.1.0