* [PATCH] nfsd: handle seqid wraparound in nfsd4_preprocess_layout_stateid
@ 2016-05-05 10:53 Jeff Layton
2016-05-05 11:00 ` Jeff Layton
2016-05-05 14:00 ` Christoph Hellwig
0 siblings, 2 replies; 4+ messages in thread
From: Jeff Layton @ 2016-05-05 10:53 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs, Christoph Hellwig
Move the existing static function to an inline helper, and call it.
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
---
fs/nfsd/nfs4layouts.c | 2 +-
fs/nfsd/nfs4state.c | 8 +-------
fs/nfsd/state.h | 5 +++++
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 825c7bc8d789..953c0755cb37 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -289,7 +289,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
status = nfserr_bad_stateid;
mutex_lock(&ls->ls_mutex);
- if (stateid->si_generation > stid->sc_stateid.si_generation)
+ if (nfsd4_stateid_generation_after(stateid, &stid->sc_stateid))
goto out_unlock_stid;
if (layout_type != ls->ls_layout_type)
goto out_unlock_stid;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0462eeddfff9..f5f82e145018 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4651,12 +4651,6 @@ grace_disallows_io(struct net *net, struct inode *inode)
return opens_in_grace(net) && mandatory_lock(inode);
}
-/* Returns true iff a is later than b: */
-static bool stateid_generation_after(stateid_t *a, stateid_t *b)
-{
- return (s32)(a->si_generation - b->si_generation) > 0;
-}
-
static __be32 check_stateid_generation(stateid_t *in, stateid_t *ref, bool has_session)
{
/*
@@ -4670,7 +4664,7 @@ static __be32 check_stateid_generation(stateid_t *in, stateid_t *ref, bool has_s
return nfs_ok;
/* If the client sends us a stateid from the future, it's buggy: */
- if (stateid_generation_after(in, ref))
+ if (nfsd4_stateid_generation_after(in, ref))
return nfserr_bad_stateid;
/*
* However, we could see a stateid from the past, even from a
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index c050c53036a6..986e51e5ceac 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -573,6 +573,11 @@ enum nfsd4_cb_op {
NFSPROC4_CLNT_CB_SEQUENCE,
};
+/* Returns true iff a is later than b: */
+static inline bool nfsd4_stateid_generation_after(stateid_t *a, stateid_t *b)
+{
+ return (s32)(a->si_generation - b->si_generation) > 0;
+}
struct nfsd4_compound_state;
struct nfsd_net;
--
2.5.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] nfsd: handle seqid wraparound in nfsd4_preprocess_layout_stateid
2016-05-05 10:53 [PATCH] nfsd: handle seqid wraparound in nfsd4_preprocess_layout_stateid Jeff Layton
@ 2016-05-05 11:00 ` Jeff Layton
2016-05-13 19:43 ` J. Bruce Fields
2016-05-05 14:00 ` Christoph Hellwig
1 sibling, 1 reply; 4+ messages in thread
From: Jeff Layton @ 2016-05-05 11:00 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs, Christoph Hellwig
On Thu, 2016-05-05 at 06:53 -0400, Jeff Layton wrote:
> Move the existing static function to an inline helper, and call it.
>
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
This is just something I noticed by inspection while ensuring that some
client-side changes that I have queued won't cause a problem. I don't
have a knfsd pnfs test rig though, so I can't test this easily.
As a side note though...should we also be verifying that the seqid is not zero in this function? That's never allowed for layout stateids.
> ---
> fs/nfsd/nfs4layouts.c | 2 +-
> fs/nfsd/nfs4state.c | 8 +-------
> fs/nfsd/state.h | 5 +++++
> 3 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> index 825c7bc8d789..953c0755cb37 100644
> --- a/fs/nfsd/nfs4layouts.c
> +++ b/fs/nfsd/nfs4layouts.c
> @@ -289,7 +289,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
>
> status = nfserr_bad_stateid;
> mutex_lock(&ls->ls_mutex);
> - if (stateid->si_generation > stid->sc_stateid.si_generation)
> + if (nfsd4_stateid_generation_after(stateid, &stid->sc_stateid))
> goto out_unlock_stid;
> if (layout_type != ls->ls_layout_type)
> goto out_unlock_stid;
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 0462eeddfff9..f5f82e145018 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4651,12 +4651,6 @@ grace_disallows_io(struct net *net, struct inode *inode)
> return opens_in_grace(net) && mandatory_lock(inode);
> }
>
> -/* Returns true iff a is later than b: */
> -static bool stateid_generation_after(stateid_t *a, stateid_t *b)
> -{
> - return (s32)(a->si_generation - b->si_generation) > 0;
> -}
> -
> static __be32 check_stateid_generation(stateid_t *in, stateid_t *ref, bool has_session)
> {
> /*
> @@ -4670,7 +4664,7 @@ static __be32 check_stateid_generation(stateid_t *in, stateid_t *ref, bool has_s
> return nfs_ok;
>
> /* If the client sends us a stateid from the future, it's buggy: */
> - if (stateid_generation_after(in, ref))
> + if (nfsd4_stateid_generation_after(in, ref))
> return nfserr_bad_stateid;
> /*
> * However, we could see a stateid from the past, even from a
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index c050c53036a6..986e51e5ceac 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -573,6 +573,11 @@ enum nfsd4_cb_op {
> NFSPROC4_CLNT_CB_SEQUENCE,
> };
>
> +/* Returns true iff a is later than b: */
> +static inline bool nfsd4_stateid_generation_after(stateid_t *a, stateid_t *b)
> +{
> + return (s32)(a->si_generation - b->si_generation) > 0;
> +}
>
> struct nfsd4_compound_state;
> struct nfsd_net;
--
Jeff Layton <jlayton@poochiereds.net>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfsd: handle seqid wraparound in nfsd4_preprocess_layout_stateid
2016-05-05 10:53 [PATCH] nfsd: handle seqid wraparound in nfsd4_preprocess_layout_stateid Jeff Layton
2016-05-05 11:00 ` Jeff Layton
@ 2016-05-05 14:00 ` Christoph Hellwig
1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2016-05-05 14:00 UTC (permalink / raw)
To: Jeff Layton; +Cc: bfields, linux-nfs, Christoph Hellwig
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfsd: handle seqid wraparound in nfsd4_preprocess_layout_stateid
2016-05-05 11:00 ` Jeff Layton
@ 2016-05-13 19:43 ` J. Bruce Fields
0 siblings, 0 replies; 4+ messages in thread
From: J. Bruce Fields @ 2016-05-13 19:43 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-nfs, Christoph Hellwig
On Thu, May 05, 2016 at 07:00:17AM -0400, Jeff Layton wrote:
> On Thu, 2016-05-05 at 06:53 -0400, Jeff Layton wrote:
> > Move the existing static function to an inline helper, and call it.
> >
> > Cc: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
>
> This is just something I noticed by inspection while ensuring that some
> client-side changes that I have queued won't cause a problem. I don't
> have a knfsd pnfs test rig though, so I can't test this easily.
>
> As a side note though...should we also be verifying that the seqid is not zero in this function? That's never allowed for layout stateids.
In nfsd4_preprocess_layout_stateid()? Sure. I guess it's just a
courtesy to help debug out-of-spec clients, but, may as well.
--b.
>
> > ---
> > fs/nfsd/nfs4layouts.c | 2 +-
> > fs/nfsd/nfs4state.c | 8 +-------
> > fs/nfsd/state.h | 5 +++++
> > 3 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> > index 825c7bc8d789..953c0755cb37 100644
> > --- a/fs/nfsd/nfs4layouts.c
> > +++ b/fs/nfsd/nfs4layouts.c
> > @@ -289,7 +289,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
> >
> > status = nfserr_bad_stateid;
> > mutex_lock(&ls->ls_mutex);
> > - if (stateid->si_generation > stid->sc_stateid.si_generation)
> > + if (nfsd4_stateid_generation_after(stateid, &stid->sc_stateid))
> > goto out_unlock_stid;
> > if (layout_type != ls->ls_layout_type)
> > goto out_unlock_stid;
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 0462eeddfff9..f5f82e145018 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -4651,12 +4651,6 @@ grace_disallows_io(struct net *net, struct inode *inode)
> > return opens_in_grace(net) && mandatory_lock(inode);
> > }
> >
> > -/* Returns true iff a is later than b: */
> > -static bool stateid_generation_after(stateid_t *a, stateid_t *b)
> > -{
> > - return (s32)(a->si_generation - b->si_generation) > 0;
> > -}
> > -
> > static __be32 check_stateid_generation(stateid_t *in, stateid_t *ref, bool has_session)
> > {
> > /*
> > @@ -4670,7 +4664,7 @@ static __be32 check_stateid_generation(stateid_t *in, stateid_t *ref, bool has_s
> > return nfs_ok;
> >
> > /* If the client sends us a stateid from the future, it's buggy: */
> > - if (stateid_generation_after(in, ref))
> > + if (nfsd4_stateid_generation_after(in, ref))
> > return nfserr_bad_stateid;
> > /*
> > * However, we could see a stateid from the past, even from a
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index c050c53036a6..986e51e5ceac 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -573,6 +573,11 @@ enum nfsd4_cb_op {
> > NFSPROC4_CLNT_CB_SEQUENCE,
> > };
> >
> > +/* Returns true iff a is later than b: */
> > +static inline bool nfsd4_stateid_generation_after(stateid_t *a, stateid_t *b)
> > +{
> > + return (s32)(a->si_generation - b->si_generation) > 0;
> > +}
> >
> > struct nfsd4_compound_state;
> > struct nfsd_net;
> --
> Jeff Layton <jlayton@poochiereds.net>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-05-13 19:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-05 10:53 [PATCH] nfsd: handle seqid wraparound in nfsd4_preprocess_layout_stateid Jeff Layton
2016-05-05 11:00 ` Jeff Layton
2016-05-13 19:43 ` J. Bruce Fields
2016-05-05 14:00 ` Christoph Hellwig
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).