linux-kernel - Re: [Ecryptfs-devel] [PATCH] ecryptfs: some inode attrs, and a question

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: <10085.1232093078@jrobl>
Date:	Fri, 16 Jan 2009 17:04:38 +0900
From:	hooanon05@...oo.co.jp
To:	Tyler Hicks <tyhicks@...ux.vnet.ibm.com>
Cc:	Dave Kleikamp <shaggy@...ux.vnet.ibm.com>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	linux-kernel@...r.kernel.org, ecryptfs-devel@...ts.launchpad.net
Subject: Re: [Ecryptfs-devel] [PATCH] ecryptfs: some inode attrs, and a question 


Tyler Hicks:
> >> - remove the unnecessary d_drop()s in ecryptfs_link().
> 
> I tend to agree that they look unnecessary, but one of them was added
> for cifs (see ae56fb16) and the other two seem to be intentional (see
> 45ec4aba) as well.  I'm working on adding support for networked
> filesystems, I'll keep these d_drop()s in mind and see if we can drop
> them. :)

commit ae56fb16337c882c52806508f93ead4034004c7a
    When CIFS is the lower filesystem, the old lower dentry needs to be explicitly
    dropped from inside eCryptfs to force a revalidate.  In addition, when CIFS is
    the lower filesystem, the inode attributes need to be copied back up from the
    lower inode to the eCryptfs inode on an eCryptfs revalidate.

Even with this commit, fstat(2) still returns incorrect attributes,
doesn't it? Because fstat(2) doesn't involve revalidate.
{
	fd = open(fileA);
	link(fileA, fileB);
	fstat(fd);
}


commit 45ec4ababe999cb95f9c0cad03b2689cb0b77a2b
    Fix the use of dget/dput calls to balance out on the lower filesystem.
	:::
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 2f2c6cf..ff4865d 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
	:::
@@ -475,8 +470,8 @@ out_lock:
 	unlock_dir(lower_dir_dentry);
 	dput(lower_new_dentry);
 	dput(lower_old_dentry);
-	if (!new_dentry->d_inode)
-		d_drop(new_dentry);
+	d_drop(new_dentry);
+	d_drop(old_dentry);
 	return rc;
 }
 
Are these two d_drop()s really intentional?
I don't understand why they are necessary after the success of link.
It might be the same to the commit ae56fb16 (see above), ie. wanting to
force re-lookup and get the latest inode attributes. (Just a guess)
Actually they are maintained in ecryptfs_link() correctly...


J. R. Okajima
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/