Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1377039 > unrolled thread

[PATCH 1/3] fscrypto: use dget_parent() in fscrypt_d_revalidate()

Started byJaegeuk Kim <jaegeuk@kernel.org>
First post2016-04-12 19:30 +0200
Last post2016-04-13 01:40 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/3] fscrypto: use dget_parent() in fscrypt_d_revalidate() Jaegeuk Kim <jaegeuk@kernel.org> - 2016-04-12 19:30 +0200
    [PATCH 2/3] f2fs: use dget_parent and file_dentry in f2fs_file_open Jaegeuk Kim <jaegeuk@kernel.org> - 2016-04-12 19:30 +0200
    Re: [PATCH 1/3] fscrypto: use dget_parent() in fscrypt_d_revalidate() Al Viro <viro@ZenIV.linux.org.uk> - 2016-04-13 00:40 +0200
      Re: [PATCH 1/3] fscrypto: use dget_parent() in fscrypt_d_revalidate() Jaegeuk Kim <jaegeuk@kernel.org> - 2016-04-13 01:40 +0200

#1377039 — [PATCH 1/3] fscrypto: use dget_parent() in fscrypt_d_revalidate()

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-04-12 19:30 +0200
Subject[PATCH 1/3] fscrypto: use dget_parent() in fscrypt_d_revalidate()
Message-ID<rnait-2Gv-3@gated-at.bofh.it>
This patch updates fscrypto along with the below ext4 crypto change.

Fixes: 3d43bcfef5f0 ("ext4 crypto: use dget_parent() in ext4_d_revalidate()")
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/crypto/crypto.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 7f58045..58ae0ba 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -345,13 +345,17 @@ EXPORT_SYMBOL(fscrypt_zeroout_range);
  */
 static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 {
-	struct inode *dir = d_inode(dentry->d_parent);
-	struct fscrypt_info *ci = dir->i_crypt_info;
+	struct dentry *dir;
+	struct fscrypt_info *ci;
 	int dir_has_key, cached_with_key;
 
-	if (!dir->i_sb->s_cop->is_encrypted(dir))
+	dir = dget_parent(dentry);
+	if (!d_inode(dir)->i_sb->s_cop->is_encrypted(d_inode(dir))) {
+		dput(dir);
 		return 0;
+	}
 
+	ci = d_inode(dir)->i_crypt_info;
 	if (ci && ci->ci_keyring_key &&
 	    (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
 					  (1 << KEY_FLAG_REVOKED) |
@@ -363,6 +367,7 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 	cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY;
 	spin_unlock(&dentry->d_lock);
 	dir_has_key = (ci != NULL);
+	dput(dir);
 
 	/*
 	 * If the dentry was cached without the key, and it is a
-- 
2.6.3

[toc] | [next] | [standalone]


#1377040 — [PATCH 2/3] f2fs: use dget_parent and file_dentry in f2fs_file_open

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-04-12 19:30 +0200
Subject[PATCH 2/3] f2fs: use dget_parent and file_dentry in f2fs_file_open
Message-ID<rnaiu-2Gv-11@gated-at.bofh.it>
In reply to#1377039
This patch synced with the below two ext4 crypto fixes together.

In 4.6-rc1, f2fs newly introduced accessing f_path.dentry which crashes
overlayfs. To fix, now we need to use file_dentry() to access that field.

Fixes: c0a37d487884 ("ext4: use file_dentry()")
Fixes: 9dd78d8c9a7b ("ext4: use dget_parent() in ext4_file_open()")
Cc: Miklos Szeredi <mszeredi@redhat.com>
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/file.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 443e077..90d1157 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -441,7 +441,7 @@ static int f2fs_file_mmap(struct file *file, struct vm_area_struct *vma)
 static int f2fs_file_open(struct inode *inode, struct file *filp)
 {
 	int ret = generic_file_open(inode, filp);
-	struct inode *dir = filp->f_path.dentry->d_parent->d_inode;
+	struct dentry *dir;
 
 	if (!ret && f2fs_encrypted_inode(inode)) {
 		ret = fscrypt_get_encryption_info(inode);
@@ -450,9 +450,13 @@ static int f2fs_file_open(struct inode *inode, struct file *filp)
 		if (!fscrypt_has_encryption_key(inode))
 			return -ENOKEY;
 	}
-	if (f2fs_encrypted_inode(dir) &&
-			!fscrypt_has_permitted_context(dir, inode))
+	dir = dget_parent(file_dentry(filp));
+	if (f2fs_encrypted_inode(d_inode(dir)) &&
+			!fscrypt_has_permitted_context(d_inode(dir), inode)) {
+		dput(dir);
 		return -EPERM;
+	}
+	dput(dir);
 	return ret;
 }
 
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]


#1377366

FromAl Viro <viro@ZenIV.linux.org.uk>
Date2016-04-13 00:40 +0200
Message-ID<rnf8u-6Qp-9@gated-at.bofh.it>
In reply to#1377039
On Tue, Apr 12, 2016 at 10:27:20AM -0700, Jaegeuk Kim wrote:
> This patch updates fscrypto along with the below ext4 crypto change.
> 
> Fixes: 3d43bcfef5f0 ("ext4 crypto: use dget_parent() in ext4_d_revalidate()")

>  static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
>  {
> -	struct inode *dir = d_inode(dentry->d_parent);
> -	struct fscrypt_info *ci = dir->i_crypt_info;
> +	struct dentry *dir;
> +	struct fscrypt_info *ci;
>  	int dir_has_key, cached_with_key;
>  
> -	if (!dir->i_sb->s_cop->is_encrypted(dir))
> +	dir = dget_parent(dentry);
> +	if (!d_inode(dir)->i_sb->s_cop->is_encrypted(d_inode(dir))) {
> +		dput(dir);

... and as soon as you call it from RCU mode, you are screwed.

[toc] | [prev] | [next] | [standalone]


#1377408

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-04-13 01:40 +0200
Message-ID<rng4x-7GD-7@gated-at.bofh.it>
In reply to#1377366
On Tue, Apr 12, 2016 at 11:33:03PM +0100, Al Viro wrote:
> On Tue, Apr 12, 2016 at 10:27:20AM -0700, Jaegeuk Kim wrote:
> > This patch updates fscrypto along with the below ext4 crypto change.
> > 
> > Fixes: 3d43bcfef5f0 ("ext4 crypto: use dget_parent() in ext4_d_revalidate()")
> 
> >  static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
> >  {
> > -	struct inode *dir = d_inode(dentry->d_parent);
> > -	struct fscrypt_info *ci = dir->i_crypt_info;
> > +	struct dentry *dir;
> > +	struct fscrypt_info *ci;
> >  	int dir_has_key, cached_with_key;
> >  
> > -	if (!dir->i_sb->s_cop->is_encrypted(dir))
> > +	dir = dget_parent(dentry);
> > +	if (!d_inode(dir)->i_sb->s_cop->is_encrypted(d_inode(dir))) {
> > +		dput(dir);
> 
> ... and as soon as you call it from RCU mode, you are screwed.

Thank you for pointing this out.
IIUC, did you mean this?

Thanks,

From a0ac7972189b7c366720a0b456e39516d622a6d4 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 12 Apr 2016 16:05:36 -0700
Subject: [PATCH] ext4/fscrypto: avoid RCU lookup in d_revalidate

As Al pointed, d_revalidate should return RCU lookup before using d_inode.
This was originally introduced by:
commit 34286d666230 ("fs: rcu-walk aware d_revalidate method").

Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Theodore Ts'o <tytso@mit.edu>
Cc: stable <stable@vger.kernel.org>
---
 fs/crypto/crypto.c | 4 ++++
 fs/ext4/crypto.c   | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index da70520..2fc8c43 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -26,6 +26,7 @@
 #include <linux/ratelimit.h>
 #include <linux/bio.h>
 #include <linux/dcache.h>
+#include <linux/namei.h>
 #include <linux/fscrypto.h>
 #include <linux/ecryptfs.h>
 
@@ -353,6 +354,9 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 	struct fscrypt_info *ci;
 	int dir_has_key, cached_with_key;
 
+	if (flags & LOOKUP_RCU)
+		return -ECHILD;
+
 	dir = dget_parent(dentry);
 	if (!d_inode(dir)->i_sb->s_cop->is_encrypted(d_inode(dir))) {
 		dput(dir);
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c
index db9ae6e..6a6c273 100644
--- a/fs/ext4/crypto.c
+++ b/fs/ext4/crypto.c
@@ -32,6 +32,7 @@
 #include <linux/random.h>
 #include <linux/scatterlist.h>
 #include <linux/spinlock_types.h>
+#include <linux/namei.h>
 
 #include "ext4_extents.h"
 #include "xattr.h"
@@ -482,6 +483,9 @@ static int ext4_d_revalidate(struct dentry *dentry, unsigned int flags)
 	struct ext4_crypt_info *ci;
 	int dir_has_key, cached_with_key;
 
+	if (flags & LOOKUP_RCU)
+		return -ECHILD;
+
 	dir = dget_parent(dentry);
 	if (!ext4_encrypted_inode(d_inode(dir))) {
 		dput(dir);
-- 
2.6.3

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web