[PATCH 1/1] OverlayFS: Fix checking permissions during lookup.

[PATCH 1/1] OverlayFS: Fix checking permissions during lookup.

[Ignacy Gawędzki]

Add alternate lookup_one_len_check function to fs/namei.c which does
what lookup_one_len did until now with a boolean argument telling
whether to check that the base directory is traversable.  Modify
original lookup_one_len function to call the former with true as the
last argument.

In function ovl_lookup_real, file fs/overlayfs/super.c, call
lookup_one_len_check with false as the last argument, so that failure
to traverse the base directory does not return -EACCES.  This should
make lookup resolution work properly in the following setup

  drwxr-xr-x lower/
  drwx------ lower/foo/
  drw-r--r-- lower/boo/bar
  drwxr-xr-x upper/
  drwxr-xr-x upper/foo/

when any user not being the owner of lower/foo is trying to access
foo/bar in the mounted overlay.

Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
---
 fs/namei.c            | 21 ++++++++++++++++++++-
 fs/overlayfs/super.c  |  2 +-
 include/linux/namei.h |  1 +
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index f624d13..1d0aedb 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2278,6 +2278,25 @@ EXPORT_SYMBOL(vfs_path_lookup);
  */
 struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
 {
+	return lookup_one_len_check(name, base, len, true);
+}
+EXPORT_SYMBOL(lookup_one_len);
+
+/**
+ * lookup_one_len_check - filesystem helper to lookup single pathname component
+ * @name:	pathname component to lookup
+ * @base:	base directory to lookup from
+ * @len:	maximum length @len should be interpreted to
+ * @check:	whether to check that @base is itself traversable.
+ *
+ * Note that this routine is purely a helper for filesystem usage and should
+ * not be called by generic code.
+ *
+ * The caller must hold base->i_mutex.
+ */
+struct dentry *lookup_one_len_check(const char *name, struct dentry *base,
+				    int len, bool check_base)
+{
 	struct qstr this;
 	unsigned int c;
 	int err;
@@ -2316,7 +2335,7 @@ struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
 
 	return __lookup_hash(&this, base, 0);
 }
-EXPORT_SYMBOL(lookup_one_len);
+EXPORT_SYMBOL(lookup_one_len_check);
 
 /**
  * lookup_one_len_unlocked - filesystem helper to lookup single pathname component
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 8d826bd..f7a1c84 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -376,7 +376,7 @@ static inline struct dentry *ovl_lookup_real(struct dentry *dir,
 	struct dentry *dentry;
 
 	inode_lock(dir->d_inode);
-	dentry = lookup_one_len(name->name, dir, name->len);
+	dentry = lookup_one_len(name->name, dir, name->len, false);
 	inode_unlock(dir->d_inode);
 
 	if (IS_ERR(dentry)) {
diff --git a/include/linux/namei.h b/include/linux/namei.h
index d0f25d8..0af6775 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -77,6 +77,7 @@ extern struct dentry *kern_path_locked(const char *, struct path *);
 extern int kern_path_mountpoint(int, const char *, struct path *, unsigned int);
 
 extern struct dentry *lookup_one_len(const char *, struct dentry *, int);
+extern struct dentry *lookup_one_len_check(const char *, struct dentry *, int, bool);
 extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int);
 
 extern int follow_down_one(struct path *);
-- 
2.5.0

Re: [PATCH 1/1] OverlayFS: Fix checking permissions during lookup.

[Ignacy Gawędzki]

On Tue, Jan 26, 2016 at 09:38:34AM +0100, thus spake Ignacy Gawedzki:
> Add alternate lookup_one_len_check function to fs/namei.c which does
> what lookup_one_len did until now with a boolean argument telling
> whether to check that the base directory is traversable.  Modify
> original lookup_one_len function to call the former with true as the
> last argument.
> 
> In function ovl_lookup_real, file fs/overlayfs/super.c, call
> lookup_one_len_check with false as the last argument, so that failure
> to traverse the base directory does not return -EACCES.  This should
> make lookup resolution work properly in the following setup
> 
>   drwxr-xr-x lower/
>   drwx------ lower/foo/
>   drw-r--r-- lower/boo/bar
>   drwxr-xr-x upper/
>   drwxr-xr-x upper/foo/
> 
> when any user not being the owner of lower/foo is trying to access
> foo/bar in the mounted overlay.

Hi everyone,

Please forgive my insisting, but I'm really looking forward to seeing
some confirmation that this is the right way to go.

Could someone take a look at this patch and confirm this is not going
to wreak havoc?

Thanks.

Ignacy

-- 
Ignacy Gawędzki
R&D Engineer
Green Communications

[PATCH 1/1] OverlayFS: Fix checking permissions during lookup.

[Ignacy Gawędzki]

Add alternate lookup_one_len_check function to fs/namei.c which does
what lookup_one_len did until now with a boolean argument telling
whether to check that the base directory is traversable.  Modify
original lookup_one_len function to call the former with true as the
last argument.

In function ovl_lookup_real, file fs/overlayfs/super.c, call
lookup_one_len_check with false as the last argument, so that failure
to traverse the base directory does not return -EACCES.  This should
make lookup resolution work properly in the following setup

  drwxr-xr-x lower/
  drwx------ lower/foo/
  drw-r--r-- lower/boo/bar
  drwxr-xr-x upper/
  drwxr-xr-x upper/foo/

when any user not being the owner of lower/foo is trying to access
foo/bar in the mounted overlay.

Signed-off-by: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr>
---
 fs/namei.c            | 21 ++++++++++++++++++++-
 fs/overlayfs/super.c  |  2 +-
 include/linux/namei.h |  1 +
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index f624d13..1d0aedb 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2278,6 +2278,25 @@ EXPORT_SYMBOL(vfs_path_lookup);
  */
 struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
 {
+	return lookup_one_len_check(name, base, len, true);
+}
+EXPORT_SYMBOL(lookup_one_len);
+
+/**
+ * lookup_one_len_check - filesystem helper to lookup single pathname component
+ * @name:	pathname component to lookup
+ * @base:	base directory to lookup from
+ * @len:	maximum length @len should be interpreted to
+ * @check:	whether to check that @base is itself traversable.
+ *
+ * Note that this routine is purely a helper for filesystem usage and should
+ * not be called by generic code.
+ *
+ * The caller must hold base->i_mutex.
+ */
+struct dentry *lookup_one_len_check(const char *name, struct dentry *base,
+				    int len, bool check_base)
+{
 	struct qstr this;
 	unsigned int c;
 	int err;
@@ -2316,7 +2335,7 @@ struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
 
 	return __lookup_hash(&this, base, 0);
 }
-EXPORT_SYMBOL(lookup_one_len);
+EXPORT_SYMBOL(lookup_one_len_check);
 
 /**
  * lookup_one_len_unlocked - filesystem helper to lookup single pathname component
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 8d826bd..f7a1c84 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -376,7 +376,7 @@ static inline struct dentry *ovl_lookup_real(struct dentry *dir,
 	struct dentry *dentry;
 
 	inode_lock(dir->d_inode);
-	dentry = lookup_one_len(name->name, dir, name->len);
+	dentry = lookup_one_len(name->name, dir, name->len, false);
 	inode_unlock(dir->d_inode);
 
 	if (IS_ERR(dentry)) {
diff --git a/include/linux/namei.h b/include/linux/namei.h
index d0f25d8..0af6775 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -77,6 +77,7 @@ extern struct dentry *kern_path_locked(const char *, struct path *);
 extern int kern_path_mountpoint(int, const char *, struct path *, unsigned int);
 
 extern struct dentry *lookup_one_len(const char *, struct dentry *, int);
+extern struct dentry *lookup_one_len_check(const char *, struct dentry *, int, bool);
 extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int);
 
 extern int follow_down_one(struct path *);
-- 
2.5.0