* [PATCH] ovl: set i_ino to the value of st_ino for NFS export
@ 2018-03-16 7:39 Amir Goldstein
2018-03-28 14:08 ` Amir Goldstein
0 siblings, 1 reply; 3+ messages in thread
From: Amir Goldstein @ 2018-03-16 7:39 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Eddie Horng, Trond Myklebust, Jeff Layton, J . Bruce Fields,
linux-unionfs
Eddie Horng reported that readdir of an overlayfs directory that
was exported via NFSv3 returns entries with d_type set to DT_UNKNOWN.
The reason is that while preparing the response for readdirplus, nfsd
checks inside encode_entryplus_baggage() that a child dentry's inode
number matches the value of d_ino returns by overlayfs readdir iterator.
Because the overlayfs inodes use arbitrary inode numbers that are not
correlated with the values of st_ino/d_ino, NFSv3 falls back to not
encoding d_type. Although this is an allowed behavior, we can fix it for
the case of all overlayfs layers on the same underlying filesystem.
When NFS export is enabled and d_ino is consistent with st_ino
(samefs), set the same value also to i_ino in ovl_fill_inode() for all
overlayfs inodes, nfsd readdirplus sanity checks will pass.
ovl_fill_inode() may be called from ovl_new_inode(), before real inode
was created with ino arg 0. In that case, i_ino will be updated to real
upper inode i_ino on ovl_inode_init() or ovl_inode_update().
Reported-by: Eddie Horng <eddiehorng.tw@gmail.com>
Tested-by: Eddie Horng <eddiehorng.tw@gmail.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/overlayfs/inode.c | 21 +++++++++++++++++----
fs/overlayfs/util.c | 8 +++++++-
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 3b1bd469accd..4689716f23d8 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -459,9 +459,20 @@ static inline void ovl_lockdep_annotate_inode_mutex_key(struct inode *inode)
#endif
}
-static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev)
+static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev,
+ unsigned long ino)
{
- inode->i_ino = get_next_ino();
+ /*
+ * When NFS export is enabled and d_ino is consistent with st_ino
+ * (samefs), set the same value to i_ino, because nfsd readdirplus
+ * compares d_ino values to i_ino values of child entries. When called
+ * from ovl_new_inode(), ino arg is 0, so i_ino will be updated to real
+ * upper inode i_ino on ovl_inode_init() or ovl_inode_update().
+ */
+ if (inode->i_sb->s_export_op && ovl_same_sb(inode->i_sb))
+ inode->i_ino = ino;
+ else
+ inode->i_ino = get_next_ino();
inode->i_mode = mode;
inode->i_flags |= S_NOCMTIME;
#ifdef CONFIG_FS_POSIX_ACL
@@ -597,7 +608,7 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, dev_t rdev)
inode = new_inode(sb);
if (inode)
- ovl_fill_inode(inode, mode, rdev);
+ ovl_fill_inode(inode, mode, rdev, 0);
return inode;
}
@@ -710,6 +721,7 @@ struct inode *ovl_get_inode(struct super_block *sb, struct dentry *upperdentry,
struct inode *inode;
bool bylower = ovl_hash_bylower(sb, upperdentry, lowerdentry, index);
bool is_dir;
+ unsigned long ino = 0;
if (!realinode)
realinode = d_inode(lowerdentry);
@@ -748,13 +760,14 @@ struct inode *ovl_get_inode(struct super_block *sb, struct dentry *upperdentry,
if (!is_dir)
nlink = ovl_get_nlink(lowerdentry, upperdentry, nlink);
set_nlink(inode, nlink);
+ ino = key->i_ino;
} else {
/* Lower hardlink that will be broken on copy up */
inode = new_inode(sb);
if (!inode)
goto out_nomem;
}
- ovl_fill_inode(inode, realinode->i_mode, realinode->i_rdev);
+ ovl_fill_inode(inode, realinode->i_mode, realinode->i_rdev, ino);
ovl_inode_init(inode, upperdentry, lowerdentry);
if (upperdentry && ovl_is_impuredir(upperdentry))
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 930784a26623..493f9b76fbf6 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -279,12 +279,16 @@ void ovl_dentry_set_redirect(struct dentry *dentry, const char *redirect)
void ovl_inode_init(struct inode *inode, struct dentry *upperdentry,
struct dentry *lowerdentry)
{
+ struct inode *realinode = d_inode(upperdentry ?: lowerdentry);
+
if (upperdentry)
OVL_I(inode)->__upperdentry = upperdentry;
if (lowerdentry)
OVL_I(inode)->lower = igrab(d_inode(lowerdentry));
- ovl_copyattr(d_inode(upperdentry ?: lowerdentry), inode);
+ ovl_copyattr(realinode, inode);
+ if (!inode->i_ino)
+ inode->i_ino = realinode->i_ino;
}
void ovl_inode_update(struct inode *inode, struct dentry *upperdentry)
@@ -299,6 +303,8 @@ void ovl_inode_update(struct inode *inode, struct dentry *upperdentry)
smp_wmb();
OVL_I(inode)->__upperdentry = upperdentry;
if (inode_unhashed(inode)) {
+ if (!inode->i_ino)
+ inode->i_ino = upperinode->i_ino;
inode->i_private = upperinode;
__insert_inode_hash(inode, (unsigned long) upperinode);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ovl: set i_ino to the value of st_ino for NFS export
2018-03-16 7:39 [PATCH] ovl: set i_ino to the value of st_ino for NFS export Amir Goldstein
@ 2018-03-28 14:08 ` Amir Goldstein
2018-03-29 9:53 ` Miklos Szeredi
0 siblings, 1 reply; 3+ messages in thread
From: Amir Goldstein @ 2018-03-28 14:08 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Eddie Horng, Trond Myklebust, Jeff Layton, J . Bruce Fields, overlayfs
Miklos,
Ping. for rc7?
If you are going to queue this patch for next, please remember to add a
cc: stable tag on it.
Thanks,
Amir.
On Fri, Mar 16, 2018 at 9:39 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> Eddie Horng reported that readdir of an overlayfs directory that
> was exported via NFSv3 returns entries with d_type set to DT_UNKNOWN.
> The reason is that while preparing the response for readdirplus, nfsd
> checks inside encode_entryplus_baggage() that a child dentry's inode
> number matches the value of d_ino returns by overlayfs readdir iterator.
>
> Because the overlayfs inodes use arbitrary inode numbers that are not
> correlated with the values of st_ino/d_ino, NFSv3 falls back to not
> encoding d_type. Although this is an allowed behavior, we can fix it for
> the case of all overlayfs layers on the same underlying filesystem.
>
> When NFS export is enabled and d_ino is consistent with st_ino
> (samefs), set the same value also to i_ino in ovl_fill_inode() for all
> overlayfs inodes, nfsd readdirplus sanity checks will pass.
> ovl_fill_inode() may be called from ovl_new_inode(), before real inode
> was created with ino arg 0. In that case, i_ino will be updated to real
> upper inode i_ino on ovl_inode_init() or ovl_inode_update().
>
> Reported-by: Eddie Horng <eddiehorng.tw@gmail.com>
> Tested-by: Eddie Horng <eddiehorng.tw@gmail.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/overlayfs/inode.c | 21 +++++++++++++++++----
> fs/overlayfs/util.c | 8 +++++++-
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 3b1bd469accd..4689716f23d8 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -459,9 +459,20 @@ static inline void ovl_lockdep_annotate_inode_mutex_key(struct inode *inode)
> #endif
> }
>
> -static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev)
> +static void ovl_fill_inode(struct inode *inode, umode_t mode, dev_t rdev,
> + unsigned long ino)
> {
> - inode->i_ino = get_next_ino();
> + /*
> + * When NFS export is enabled and d_ino is consistent with st_ino
> + * (samefs), set the same value to i_ino, because nfsd readdirplus
> + * compares d_ino values to i_ino values of child entries. When called
> + * from ovl_new_inode(), ino arg is 0, so i_ino will be updated to real
> + * upper inode i_ino on ovl_inode_init() or ovl_inode_update().
> + */
> + if (inode->i_sb->s_export_op && ovl_same_sb(inode->i_sb))
> + inode->i_ino = ino;
> + else
> + inode->i_ino = get_next_ino();
> inode->i_mode = mode;
> inode->i_flags |= S_NOCMTIME;
> #ifdef CONFIG_FS_POSIX_ACL
> @@ -597,7 +608,7 @@ struct inode *ovl_new_inode(struct super_block *sb, umode_t mode, dev_t rdev)
>
> inode = new_inode(sb);
> if (inode)
> - ovl_fill_inode(inode, mode, rdev);
> + ovl_fill_inode(inode, mode, rdev, 0);
>
> return inode;
> }
> @@ -710,6 +721,7 @@ struct inode *ovl_get_inode(struct super_block *sb, struct dentry *upperdentry,
> struct inode *inode;
> bool bylower = ovl_hash_bylower(sb, upperdentry, lowerdentry, index);
> bool is_dir;
> + unsigned long ino = 0;
>
> if (!realinode)
> realinode = d_inode(lowerdentry);
> @@ -748,13 +760,14 @@ struct inode *ovl_get_inode(struct super_block *sb, struct dentry *upperdentry,
> if (!is_dir)
> nlink = ovl_get_nlink(lowerdentry, upperdentry, nlink);
> set_nlink(inode, nlink);
> + ino = key->i_ino;
> } else {
> /* Lower hardlink that will be broken on copy up */
> inode = new_inode(sb);
> if (!inode)
> goto out_nomem;
> }
> - ovl_fill_inode(inode, realinode->i_mode, realinode->i_rdev);
> + ovl_fill_inode(inode, realinode->i_mode, realinode->i_rdev, ino);
> ovl_inode_init(inode, upperdentry, lowerdentry);
>
> if (upperdentry && ovl_is_impuredir(upperdentry))
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 930784a26623..493f9b76fbf6 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -279,12 +279,16 @@ void ovl_dentry_set_redirect(struct dentry *dentry, const char *redirect)
> void ovl_inode_init(struct inode *inode, struct dentry *upperdentry,
> struct dentry *lowerdentry)
> {
> + struct inode *realinode = d_inode(upperdentry ?: lowerdentry);
> +
> if (upperdentry)
> OVL_I(inode)->__upperdentry = upperdentry;
> if (lowerdentry)
> OVL_I(inode)->lower = igrab(d_inode(lowerdentry));
>
> - ovl_copyattr(d_inode(upperdentry ?: lowerdentry), inode);
> + ovl_copyattr(realinode, inode);
> + if (!inode->i_ino)
> + inode->i_ino = realinode->i_ino;
> }
>
> void ovl_inode_update(struct inode *inode, struct dentry *upperdentry)
> @@ -299,6 +303,8 @@ void ovl_inode_update(struct inode *inode, struct dentry *upperdentry)
> smp_wmb();
> OVL_I(inode)->__upperdentry = upperdentry;
> if (inode_unhashed(inode)) {
> + if (!inode->i_ino)
> + inode->i_ino = upperinode->i_ino;
> inode->i_private = upperinode;
> __insert_inode_hash(inode, (unsigned long) upperinode);
> }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ovl: set i_ino to the value of st_ino for NFS export
2018-03-28 14:08 ` Amir Goldstein
@ 2018-03-29 9:53 ` Miklos Szeredi
0 siblings, 0 replies; 3+ messages in thread
From: Miklos Szeredi @ 2018-03-29 9:53 UTC (permalink / raw)
To: Amir Goldstein
Cc: Eddie Horng, Trond Myklebust, Jeff Layton, J . Bruce Fields, overlayfs
On Wed, Mar 28, 2018 at 4:08 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Miklos,
>
> Ping. for rc7?
> If you are going to queue this patch for next, please remember to add a
> cc: stable tag on it.
Yep, added cc:stable tag.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-29 9:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-16 7:39 [PATCH] ovl: set i_ino to the value of st_ino for NFS export Amir Goldstein
2018-03-28 14:08 ` Amir Goldstein
2018-03-29 9:53 ` Miklos Szeredi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.