* [PATCH 0/3] make BTRFS, UDF, NILFS2 work with NFSv2. @ 2015-05-08 0:16 NeilBrown [not found] ` <20150508001142.31129.7604.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: NeilBrown @ 2015-05-08 0:16 UTC (permalink / raw) To: Josef Bacik, Chris Mason, Jan Kara, David Sterba, Ryusuke Konishi Cc: linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs Hi filesystem folks, I just had a report that BTRFS doesn't work reliably with NFSv2. The problem is that NFSv2 doesn't encode filehandle size so it may report to the filesystem a longer handle that is being expected. Filesystems should not require a specific length, only at least that length. A code audit shows that NILFS2 and UDF suffer the same problem. Following patches should fix it... well, they compile and look good. Please consider for inclusion is respective trees. Admittedly NFSv2 is a bit "last century", but while it is easy to support, we may as well. Thanks, NeilBrown --- NeilBrown (3): BTRFS: support NFSv2 export NILFS2: support NFSv2 export UDF: support NFSv2 export fs/btrfs/export.c | 10 +++++----- fs/nilfs2/namei.c | 6 +++--- fs/udf/namei.c | 16 ++++++++++++---- 3 files changed, 20 insertions(+), 12 deletions(-) -- Signature ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20150508001142.31129.7604.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>]
* [PATCH 1/3] BTRFS: support NFSv2 export [not found] ` <20150508001142.31129.7604.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> @ 2015-05-08 0:16 ` NeilBrown 2015-05-11 8:13 ` David Sterba 0 siblings, 1 reply; 12+ messages in thread From: NeilBrown @ 2015-05-08 0:16 UTC (permalink / raw) To: Josef Bacik, Chris Mason, Jan Kara, David Sterba, Ryusuke Konishi Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, linux-nfs-u79uwXL29TY76Z2rM5mHXA, linux-nilfs-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-btrfs-u79uwXL29TY76Z2rM5mHXA The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as that returned by encode_fh - it may be larger. With NFSv2, the filehandle is fixed length, so it may appear longer than expected and be zero-padded. So we must test that fh_len is at least some value, not exactly equal to it. Signed-off-by: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org> --- fs/btrfs/export.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c index 8d052209f473..2513a7f53334 100644 --- a/fs/btrfs/export.c +++ b/fs/btrfs/export.c @@ -112,11 +112,11 @@ static struct dentry *btrfs_fh_to_parent(struct super_block *sb, struct fid *fh, u32 generation; if (fh_type == FILEID_BTRFS_WITH_PARENT) { - if (fh_len != BTRFS_FID_SIZE_CONNECTABLE) + if (fh_len < BTRFS_FID_SIZE_CONNECTABLE) return NULL; root_objectid = fid->root_objectid; } else if (fh_type == FILEID_BTRFS_WITH_PARENT_ROOT) { - if (fh_len != BTRFS_FID_SIZE_CONNECTABLE_ROOT) + if (fh_len < BTRFS_FID_SIZE_CONNECTABLE_ROOT) return NULL; root_objectid = fid->parent_root_objectid; } else @@ -136,11 +136,11 @@ static struct dentry *btrfs_fh_to_dentry(struct super_block *sb, struct fid *fh, u32 generation; if ((fh_type != FILEID_BTRFS_WITH_PARENT || - fh_len != BTRFS_FID_SIZE_CONNECTABLE) && + fh_len < BTRFS_FID_SIZE_CONNECTABLE) && (fh_type != FILEID_BTRFS_WITH_PARENT_ROOT || - fh_len != BTRFS_FID_SIZE_CONNECTABLE_ROOT) && + fh_len < BTRFS_FID_SIZE_CONNECTABLE_ROOT) && (fh_type != FILEID_BTRFS_WITHOUT_PARENT || - fh_len != BTRFS_FID_SIZE_NON_CONNECTABLE)) + fh_len < BTRFS_FID_SIZE_NON_CONNECTABLE)) return NULL; objectid = fid->objectid; -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] BTRFS: support NFSv2 export 2015-05-08 0:16 ` [PATCH 1/3] BTRFS: support NFSv2 export NeilBrown @ 2015-05-11 8:13 ` David Sterba 2015-09-24 1:59 ` Neil Brown 0 siblings, 1 reply; 12+ messages in thread From: David Sterba @ 2015-05-11 8:13 UTC (permalink / raw) To: NeilBrown Cc: Josef Bacik, Chris Mason, Jan Kara, Ryusuke Konishi, linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs On Fri, May 08, 2015 at 10:16:23AM +1000, NeilBrown wrote: > The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as > that returned by encode_fh - it may be larger. > > With NFSv2, the filehandle is fixed length, so it may appear longer > than expected and be zero-padded. > > So we must test that fh_len is at least some value, not exactly equal > to it. > > Signed-off-by: NeilBrown <neilb@suse.de> Acked-by: David Sterba <dsterba@suse.cz> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] BTRFS: support NFSv2 export 2015-05-11 8:13 ` David Sterba @ 2015-09-24 1:59 ` Neil Brown 2015-10-05 14:32 ` Chris Mason 0 siblings, 1 reply; 12+ messages in thread From: Neil Brown @ 2015-09-24 1:59 UTC (permalink / raw) To: dsterba Cc: Josef Bacik, Chris Mason, Jan Kara, Ryusuke Konishi, linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs [-- Attachment #1: Type: text/plain, Size: 672 bytes --] David Sterba <dsterba@suse.cz> writes: > On Fri, May 08, 2015 at 10:16:23AM +1000, NeilBrown wrote: >> The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as >> that returned by encode_fh - it may be larger. >> >> With NFSv2, the filehandle is fixed length, so it may appear longer >> than expected and be zero-padded. >> >> So we must test that fh_len is at least some value, not exactly equal >> to it. >> >> Signed-off-by: NeilBrown <neilb@suse.de> > > Acked-by: David Sterba <dsterba@suse.cz> Thanks. However I just checked mainline and this still hasn't been applied. Should I resend it to someone? Who? Thanks, NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] BTRFS: support NFSv2 export 2015-09-24 1:59 ` Neil Brown @ 2015-10-05 14:32 ` Chris Mason 0 siblings, 0 replies; 12+ messages in thread From: Chris Mason @ 2015-10-05 14:32 UTC (permalink / raw) To: Neil Brown Cc: dsterba, Josef Bacik, Jan Kara, Ryusuke Konishi, linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs On Thu, Sep 24, 2015 at 11:59:02AM +1000, Neil Brown wrote: > David Sterba <dsterba@suse.cz> writes: > > > On Fri, May 08, 2015 at 10:16:23AM +1000, NeilBrown wrote: > >> The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as > >> that returned by encode_fh - it may be larger. > >> > >> With NFSv2, the filehandle is fixed length, so it may appear longer > >> than expected and be zero-padded. > >> > >> So we must test that fh_len is at least some value, not exactly equal > >> to it. > >> > >> Signed-off-by: NeilBrown <neilb@suse.de> > > > > Acked-by: David Sterba <dsterba@suse.cz> > > Thanks. However I just checked mainline and this still hasn't been > applied. > Should I resend it to someone? Who? Sorry Neil, I thought you were pushing these after it was ack'd. I'll put it in my pull this week. -chris ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] UDF: support NFSv2 export 2015-05-08 0:16 [PATCH 0/3] make BTRFS, UDF, NILFS2 work with NFSv2 NeilBrown [not found] ` <20150508001142.31129.7604.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> @ 2015-05-08 0:16 ` NeilBrown [not found] ` <20150508001623.31129.24710.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> 2015-05-08 0:16 ` [PATCH 2/3] NILFS2: " NeilBrown 2 siblings, 1 reply; 12+ messages in thread From: NeilBrown @ 2015-05-08 0:16 UTC (permalink / raw) To: Josef Bacik, Chris Mason, Jan Kara, David Sterba, Ryusuke Konishi Cc: linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as that returned by encode_fh - it may be larger. With NFSv2, the filehandle is fixed length, so it may appear longer than expected and be zero-padded. So we must test that fh_len is at least some value, not exactly equal to it. Signed-off-by: NeilBrown <neilb@suse.de> --- fs/udf/namei.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/udf/namei.c b/fs/udf/namei.c index 5c03f0dfb98b..facc2a840f7b 100644 --- a/fs/udf/namei.c +++ b/fs/udf/namei.c @@ -1221,7 +1221,7 @@ static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block, static struct dentry *udf_fh_to_dentry(struct super_block *sb, struct fid *fid, int fh_len, int fh_type) { - if ((fh_len != 3 && fh_len != 5) || + if (fh_len < 3 || (fh_type != FILEID_UDF_WITH_PARENT && fh_type != FILEID_UDF_WITHOUT_PARENT)) return NULL; @@ -1233,7 +1233,7 @@ static struct dentry *udf_fh_to_dentry(struct super_block *sb, static struct dentry *udf_fh_to_parent(struct super_block *sb, struct fid *fid, int fh_len, int fh_type) { - if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT) + if (fh_len < 5 || fh_type != FILEID_UDF_WITH_PARENT) return NULL; return udf_nfs_get_inode(sb, fid->udf.parent_block, ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <20150508001623.31129.24710.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>]
* Re: [PATCH 3/3] UDF: support NFSv2 export [not found] ` <20150508001623.31129.24710.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> @ 2015-05-11 8:57 ` Jan Kara 0 siblings, 0 replies; 12+ messages in thread From: Jan Kara @ 2015-05-11 8:57 UTC (permalink / raw) To: NeilBrown Cc: Josef Bacik, Chris Mason, Jan Kara, David Sterba, Ryusuke Konishi, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, linux-nfs-u79uwXL29TY76Z2rM5mHXA, linux-nilfs-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-btrfs-u79uwXL29TY76Z2rM5mHXA On Fri 08-05-15 10:16:23, NeilBrown wrote: > The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as > that returned by encode_fh - it may be larger. > > With NFSv2, the filehandle is fixed length, so it may appear longer > than expected and be zero-padded. > > So we must test that fh_len is at least some value, not exactly equal > to it. > > Signed-off-by: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org> Thanks. The patch looks good to me. I've added it to my tree. Honza > --- > fs/udf/namei.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/udf/namei.c b/fs/udf/namei.c > index 5c03f0dfb98b..facc2a840f7b 100644 > --- a/fs/udf/namei.c > +++ b/fs/udf/namei.c > @@ -1221,7 +1221,7 @@ static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block, > static struct dentry *udf_fh_to_dentry(struct super_block *sb, > struct fid *fid, int fh_len, int fh_type) > { > - if ((fh_len != 3 && fh_len != 5) || > + if (fh_len < 3 || > (fh_type != FILEID_UDF_WITH_PARENT && > fh_type != FILEID_UDF_WITHOUT_PARENT)) > return NULL; > @@ -1233,7 +1233,7 @@ static struct dentry *udf_fh_to_dentry(struct super_block *sb, > static struct dentry *udf_fh_to_parent(struct super_block *sb, > struct fid *fid, int fh_len, int fh_type) > { > - if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT) > + if (fh_len < 5 || fh_type != FILEID_UDF_WITH_PARENT) > return NULL; > > return udf_nfs_get_inode(sb, fid->udf.parent_block, > > -- Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] NILFS2: support NFSv2 export 2015-05-08 0:16 [PATCH 0/3] make BTRFS, UDF, NILFS2 work with NFSv2 NeilBrown [not found] ` <20150508001142.31129.7604.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> 2015-05-08 0:16 ` [PATCH 3/3] UDF: " NeilBrown @ 2015-05-08 0:16 ` NeilBrown [not found] ` <20150508001623.31129.25102.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> 2 siblings, 1 reply; 12+ messages in thread From: NeilBrown @ 2015-05-08 0:16 UTC (permalink / raw) To: Josef Bacik, Chris Mason, Jan Kara, David Sterba, Ryusuke Konishi Cc: linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as that returned by encode_fh - it may be larger. With NFSv2, the filehandle is fixed length, so it may appear longer than expected and be zero-padded. So we must test that fh_len is at least some value, not exactly equal to it. Signed-off-by: NeilBrown <neilb@suse.de> --- fs/nilfs2/namei.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c index 22180836ec22..b65fb79d16fd 100644 --- a/fs/nilfs2/namei.c +++ b/fs/nilfs2/namei.c @@ -496,8 +496,8 @@ static struct dentry *nilfs_fh_to_dentry(struct super_block *sb, struct fid *fh, { struct nilfs_fid *fid = (struct nilfs_fid *)fh; - if ((fh_len != NILFS_FID_SIZE_NON_CONNECTABLE && - fh_len != NILFS_FID_SIZE_CONNECTABLE) || + if ((fh_len < NILFS_FID_SIZE_NON_CONNECTABLE && + fh_len < NILFS_FID_SIZE_CONNECTABLE) || (fh_type != FILEID_NILFS_WITH_PARENT && fh_type != FILEID_NILFS_WITHOUT_PARENT)) return NULL; @@ -510,7 +510,7 @@ static struct dentry *nilfs_fh_to_parent(struct super_block *sb, struct fid *fh, { struct nilfs_fid *fid = (struct nilfs_fid *)fh; - if (fh_len != NILFS_FID_SIZE_CONNECTABLE || + if (fh_len < NILFS_FID_SIZE_CONNECTABLE || fh_type != FILEID_NILFS_WITH_PARENT) return NULL; ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <20150508001623.31129.25102.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>]
* Re: [PATCH 2/3] NILFS2: support NFSv2 export [not found] ` <20150508001623.31129.25102.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> @ 2015-05-10 16:31 ` Ryusuke Konishi 2015-05-11 7:02 ` NeilBrown 0 siblings, 1 reply; 12+ messages in thread From: Ryusuke Konishi @ 2015-05-10 16:31 UTC (permalink / raw) To: NeilBrown Cc: Josef Bacik, Chris Mason, Jan Kara, David Sterba, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, linux-nfs-u79uwXL29TY76Z2rM5mHXA, linux-nilfs-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-btrfs-u79uwXL29TY76Z2rM5mHXA On Fri, 08 May 2015 10:16:23 +1000, NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org> wrote: > The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as > that returned by encode_fh - it may be larger. > > With NFSv2, the filehandle is fixed length, so it may appear longer > than expected and be zero-padded. > > So we must test that fh_len is at least some value, not exactly equal > to it. > > Signed-off-by: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org> > --- > fs/nilfs2/namei.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c > index 22180836ec22..b65fb79d16fd 100644 > --- a/fs/nilfs2/namei.c > +++ b/fs/nilfs2/namei.c > @@ -496,8 +496,8 @@ static struct dentry *nilfs_fh_to_dentry(struct super_block *sb, struct fid *fh, > { > struct nilfs_fid *fid = (struct nilfs_fid *)fh; > > - if ((fh_len != NILFS_FID_SIZE_NON_CONNECTABLE && > - fh_len != NILFS_FID_SIZE_CONNECTABLE) || > + if ((fh_len < NILFS_FID_SIZE_NON_CONNECTABLE && > + fh_len < NILFS_FID_SIZE_CONNECTABLE) || > (fh_type != FILEID_NILFS_WITH_PARENT && > fh_type != FILEID_NILFS_WITHOUT_PARENT)) > return NULL; A bit weird. "fh_len < NILFS_FID_SIZE_CONNECTABLE" implies "fh_len < NILFS_FID_SIZE_NON_CONNECTABLE". How about the following fix ? if ((fh_type != FILEID_NILFS_WITH_PARENT || fh_len < NILFS_FID_SIZE_CONNECTABLE) && (fh_type != FILEID_NILFS_WITHOUT_PARENT || fh_len < NILFS_FID_SIZE_NON_CONNECTABLE)) return NULL; Regards, Ryusuke Konishi > @@ -510,7 +510,7 @@ static struct dentry *nilfs_fh_to_parent(struct super_block *sb, struct fid *fh, > { > struct nilfs_fid *fid = (struct nilfs_fid *)fh; > > - if (fh_len != NILFS_FID_SIZE_CONNECTABLE || > + if (fh_len < NILFS_FID_SIZE_CONNECTABLE || > fh_type != FILEID_NILFS_WITH_PARENT) > return NULL; > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] NILFS2: support NFSv2 export 2015-05-10 16:31 ` Ryusuke Konishi @ 2015-05-11 7:02 ` NeilBrown 2015-05-11 9:54 ` Ryusuke Konishi 0 siblings, 1 reply; 12+ messages in thread From: NeilBrown @ 2015-05-11 7:02 UTC (permalink / raw) To: Ryusuke Konishi Cc: Josef Bacik, Chris Mason, Jan Kara, David Sterba, linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs [-- Attachment #1: Type: text/plain, Size: 2446 bytes --] On Mon, 11 May 2015 01:31:43 +0900 (JST) Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp> wrote: > On Fri, 08 May 2015 10:16:23 +1000, NeilBrown <neilb@suse.de> wrote: > > The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as > > that returned by encode_fh - it may be larger. > > > > With NFSv2, the filehandle is fixed length, so it may appear longer > > than expected and be zero-padded. > > > > So we must test that fh_len is at least some value, not exactly equal > > to it. > > > > Signed-off-by: NeilBrown <neilb@suse.de> > > --- > > fs/nilfs2/namei.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c > > index 22180836ec22..b65fb79d16fd 100644 > > --- a/fs/nilfs2/namei.c > > +++ b/fs/nilfs2/namei.c > > @@ -496,8 +496,8 @@ static struct dentry *nilfs_fh_to_dentry(struct super_block *sb, struct fid *fh, > > { > > struct nilfs_fid *fid = (struct nilfs_fid *)fh; > > > > - if ((fh_len != NILFS_FID_SIZE_NON_CONNECTABLE && > > - fh_len != NILFS_FID_SIZE_CONNECTABLE) || > > > + if ((fh_len < NILFS_FID_SIZE_NON_CONNECTABLE && > > + fh_len < NILFS_FID_SIZE_CONNECTABLE) || > > (fh_type != FILEID_NILFS_WITH_PARENT && > > fh_type != FILEID_NILFS_WITHOUT_PARENT)) > > return NULL; > > A bit weird. "fh_len < NILFS_FID_SIZE_CONNECTABLE" implies "fh_len < > NILFS_FID_SIZE_NON_CONNECTABLE". > > How about the following fix ? > > if ((fh_type != FILEID_NILFS_WITH_PARENT || > fh_len < NILFS_FID_SIZE_CONNECTABLE) && > (fh_type != FILEID_NILFS_WITHOUT_PARENT || > fh_len < NILFS_FID_SIZE_NON_CONNECTABLE)) > return NULL; > Yes, weird. The code only uses the early parts of the filehandle, so we only need to complain if the fh_len is less than FILEID_NILFS_WITHOUT_PARENT. So I'd prefer: @@ -496,8 +496,7 @@ static struct dentry *nilfs_fh_to_dentry(struct super_block *sb, struct fid *fh, { struct nilfs_fid *fid = (struct nilfs_fid *)fh; - if ((fh_len != NILFS_FID_SIZE_NON_CONNECTABLE && - fh_len != NILFS_FID_SIZE_CONNECTABLE) || + if (fh_len < NILFS_FID_SIZE_NON_CONNECTABLE || (fh_type != FILEID_NILFS_WITH_PARENT && fh_type != FILEID_NILFS_WITHOUT_PARENT)) return NULL; Would you be OK with that? If so I'll resend. Thanks, NeilBrown [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] NILFS2: support NFSv2 export 2015-05-11 7:02 ` NeilBrown @ 2015-05-11 9:54 ` Ryusuke Konishi 2015-05-11 11:13 ` [PATCH v2] " NeilBrown 0 siblings, 1 reply; 12+ messages in thread From: Ryusuke Konishi @ 2015-05-11 9:54 UTC (permalink / raw) To: NeilBrown Cc: Josef Bacik, Chris Mason, Jan Kara, David Sterba, linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs On Mon, 11 May 2015 17:02:51 +1000, NeilBrown wrote: > On Mon, 11 May 2015 01:31:43 +0900 (JST) Ryusuke Konishi > <konishi.ryusuke@lab.ntt.co.jp> wrote: > >> On Fri, 08 May 2015 10:16:23 +1000, NeilBrown <neilb@suse.de> wrote: >> > The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as >> > that returned by encode_fh - it may be larger. >> > >> > With NFSv2, the filehandle is fixed length, so it may appear longer >> > than expected and be zero-padded. >> > >> > So we must test that fh_len is at least some value, not exactly equal >> > to it. >> > >> > Signed-off-by: NeilBrown <neilb@suse.de> >> > --- >> > fs/nilfs2/namei.c | 6 +++--- >> > 1 file changed, 3 insertions(+), 3 deletions(-) >> > >> > diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c >> > index 22180836ec22..b65fb79d16fd 100644 >> > --- a/fs/nilfs2/namei.c >> > +++ b/fs/nilfs2/namei.c >> > @@ -496,8 +496,8 @@ static struct dentry *nilfs_fh_to_dentry(struct super_block *sb, struct fid *fh, >> > { >> > struct nilfs_fid *fid = (struct nilfs_fid *)fh; >> > >> > - if ((fh_len != NILFS_FID_SIZE_NON_CONNECTABLE && >> > - fh_len != NILFS_FID_SIZE_CONNECTABLE) || >> >> > + if ((fh_len < NILFS_FID_SIZE_NON_CONNECTABLE && >> > + fh_len < NILFS_FID_SIZE_CONNECTABLE) || >> > (fh_type != FILEID_NILFS_WITH_PARENT && >> > fh_type != FILEID_NILFS_WITHOUT_PARENT)) >> > return NULL; >> >> A bit weird. "fh_len < NILFS_FID_SIZE_CONNECTABLE" implies "fh_len < >> NILFS_FID_SIZE_NON_CONNECTABLE". >> >> How about the following fix ? >> >> if ((fh_type != FILEID_NILFS_WITH_PARENT || >> fh_len < NILFS_FID_SIZE_CONNECTABLE) && >> (fh_type != FILEID_NILFS_WITHOUT_PARENT || >> fh_len < NILFS_FID_SIZE_NON_CONNECTABLE)) >> return NULL; >> > > Yes, weird. The code only uses the early parts of the filehandle, so we > only need to complain if the fh_len is less than FILEID_NILFS_WITHOUT_PARENT. > > So I'd prefer: > > @@ -496,8 +496,7 @@ static struct dentry *nilfs_fh_to_dentry(struct super_block *sb, struct fid *fh, > { > struct nilfs_fid *fid = (struct nilfs_fid *)fh; > > - if ((fh_len != NILFS_FID_SIZE_NON_CONNECTABLE && > - fh_len != NILFS_FID_SIZE_CONNECTABLE) || > + if (fh_len < NILFS_FID_SIZE_NON_CONNECTABLE || > (fh_type != FILEID_NILFS_WITH_PARENT && > fh_type != FILEID_NILFS_WITHOUT_PARENT)) > return NULL; > > > Would you be OK with that? If so I'll resend. > > Thanks, > NeilBrown Thanks. This looks OK to me. I'll apply it if you will resend. Regards, Ryusuke Konishi ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] NILFS2: support NFSv2 export 2015-05-11 9:54 ` Ryusuke Konishi @ 2015-05-11 11:13 ` NeilBrown 0 siblings, 0 replies; 12+ messages in thread From: NeilBrown @ 2015-05-11 11:13 UTC (permalink / raw) To: Ryusuke Konishi Cc: Josef Bacik, Chris Mason, Jan Kara, David Sterba, linux-fsdevel, linux-nfs, linux-nilfs, linux-kernel, linux-btrfs [-- Attachment #1: Type: text/plain, Size: 1238 bytes --] The "fh_len" passed to ->fh_to_* is not guaranteed to be that same as that returned by encode_fh - it may be larger. With NFSv2, the filehandle is fixed length, so it may appear longer than expected and be zero-padded. So we must test that fh_len is at least some value, not exactly equal to it. Signed-off-by: NeilBrown <neilb@suse.de> diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c index 22180836ec22..37dd6b05b1b5 100644 --- a/fs/nilfs2/namei.c +++ b/fs/nilfs2/namei.c @@ -496,8 +496,7 @@ static struct dentry *nilfs_fh_to_dentry(struct super_block *sb, struct fid *fh, { struct nilfs_fid *fid = (struct nilfs_fid *)fh; - if ((fh_len != NILFS_FID_SIZE_NON_CONNECTABLE && - fh_len != NILFS_FID_SIZE_CONNECTABLE) || + if (fh_len < NILFS_FID_SIZE_NON_CONNECTABLE || (fh_type != FILEID_NILFS_WITH_PARENT && fh_type != FILEID_NILFS_WITHOUT_PARENT)) return NULL; @@ -510,7 +509,7 @@ static struct dentry *nilfs_fh_to_parent(struct super_block *sb, struct fid *fh, { struct nilfs_fid *fid = (struct nilfs_fid *)fh; - if (fh_len != NILFS_FID_SIZE_CONNECTABLE || + if (fh_len < NILFS_FID_SIZE_CONNECTABLE || fh_type != FILEID_NILFS_WITH_PARENT) return NULL; [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-10-05 14:32 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-08 0:16 [PATCH 0/3] make BTRFS, UDF, NILFS2 work with NFSv2 NeilBrown [not found] ` <20150508001142.31129.7604.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> 2015-05-08 0:16 ` [PATCH 1/3] BTRFS: support NFSv2 export NeilBrown 2015-05-11 8:13 ` David Sterba 2015-09-24 1:59 ` Neil Brown 2015-10-05 14:32 ` Chris Mason 2015-05-08 0:16 ` [PATCH 3/3] UDF: " NeilBrown [not found] ` <20150508001623.31129.24710.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> 2015-05-11 8:57 ` Jan Kara 2015-05-08 0:16 ` [PATCH 2/3] NILFS2: " NeilBrown [not found] ` <20150508001623.31129.25102.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org> 2015-05-10 16:31 ` Ryusuke Konishi 2015-05-11 7:02 ` NeilBrown 2015-05-11 9:54 ` Ryusuke Konishi 2015-05-11 11:13 ` [PATCH v2] " NeilBrown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).