2009-06-16 01:19:37

by Benny Halevy

Subject: [PATCH 09/44] nfsd41: sanity check client drc maxreqs

From: Andy Adamson <[email protected]>

Ensure the client requested maximum requests are between 1 and
NFSD_MAX_SLOTS_PER_SESSION

Signed-off-by: Andy Adamson <[email protected]>
Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4state.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 582cd2c..90e6645 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -425,6 +425,11 @@ static int set_forechannel_maxreqs(struct nfsd4_channel_attrs *fchan)
{
int status = 0, np = fchan->maxreqs * NFSD_PAGES_PER_SLOT;

+ if (fchan->maxreqs < 1)
+ return nfserr_inval;
+ else if (fchan->maxreqs > NFSD_MAX_SLOTS_PER_SESSION)
+ fchan->maxreqs = NFSD_MAX_SLOTS_PER_SESSION;
+
spin_lock(&nfsd_serv->sv_lock);
if (np + nfsd_serv->sv_drc_pages_used > nfsd_serv->sv_drc_max_pages)
np = nfsd_serv->sv_drc_max_pages - nfsd_serv->sv_drc_pages_used;
--
1.6.3

2009-06-17 21:41:05

by J. Bruce Fields

Subject: Re: [PATCH 09/44] nfsd41: sanity check client drc maxreqs

On Tue, Jun 16, 2009 at 08:44:43PM -0400, bfields wrote:
> On Tue, Jun 16, 2009 at 08:35:38PM -0400, bfields wrote:
> > On Tue, Jun 16, 2009 at 04:19:38AM +0300, Benny Halevy wrote:
> > > From: Andy Adamson <[email protected]>
> > >
> > > Ensure the client requested maximum requests are between 1 and
> > > NFSD_MAX_SLOTS_PER_SESSION
> > >
> > > Signed-off-by: Andy Adamson <[email protected]>
> > > Signed-off-by: Benny Halevy <[email protected]>
> > > ---
> > > fs/nfsd/nfs4state.c | 5 +++++
> > > 1 files changed, 5 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > index 582cd2c..90e6645 100644
> > > --- a/fs/nfsd/nfs4state.c
> > > +++ b/fs/nfsd/nfs4state.c
> > > @@ -425,6 +425,11 @@ static int set_forechannel_maxreqs(struct nfsd4_channel_attrs *fchan)
> > > {
> > > int status = 0, np = fchan->maxreqs * NFSD_PAGES_PER_SLOT;
> > >
> > > + if (fchan->maxreqs < 1)
> > > + return nfserr_inval;
> > > + else if (fchan->maxreqs > NFSD_MAX_SLOTS_PER_SESSION)
> > > + fchan->maxreqs = NFSD_MAX_SLOTS_PER_SESSION;
> > > +
> > > spin_lock(&nfsd_serv->sv_lock);
> >
> > How did this use of sv_lock creep in? Let's remove that before we do
> > anything else....
>
> (But--applied.)

By the way, I just noticed, the caller already does the second of those
two checks; a followup patch should remove that now-superfluous code in
the caller.

--b.

2009-06-17 00:35:41

by J. Bruce Fields

Subject: Re: [PATCH 09/44] nfsd41: sanity check client drc maxreqs

On Tue, Jun 16, 2009 at 04:19:38AM +0300, Benny Halevy wrote:
> From: Andy Adamson <[email protected]>
>
> Ensure the client requested maximum requests are between 1 and
> NFSD_MAX_SLOTS_PER_SESSION
>
> Signed-off-by: Andy Adamson <[email protected]>
> Signed-off-by: Benny Halevy <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 582cd2c..90e6645 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -425,6 +425,11 @@ static int set_forechannel_maxreqs(struct nfsd4_channel_attrs *fchan)
> {
> int status = 0, np = fchan->maxreqs * NFSD_PAGES_PER_SLOT;
>
> + if (fchan->maxreqs < 1)
> + return nfserr_inval;
> + else if (fchan->maxreqs > NFSD_MAX_SLOTS_PER_SESSION)
> + fchan->maxreqs = NFSD_MAX_SLOTS_PER_SESSION;
> +
> spin_lock(&nfsd_serv->sv_lock);

How did this use of sv_lock creep in? Let's remove that before we do
anything else....

--b.

> if (np + nfsd_serv->sv_drc_pages_used > nfsd_serv->sv_drc_max_pages)
> np = nfsd_serv->sv_drc_max_pages - nfsd_serv->sv_drc_pages_used;
> --
> 1.6.3
>

2009-06-17 00:44:41

by J. Bruce Fields

Subject: Re: [PATCH 09/44] nfsd41: sanity check client drc maxreqs

On Tue, Jun 16, 2009 at 08:35:38PM -0400, bfields wrote:
> On Tue, Jun 16, 2009 at 04:19:38AM +0300, Benny Halevy wrote:
> > From: Andy Adamson <[email protected]>
> >
> > Ensure the client requested maximum requests are between 1 and
> > NFSD_MAX_SLOTS_PER_SESSION
> >
> > Signed-off-by: Andy Adamson <[email protected]>
> > Signed-off-by: Benny Halevy <[email protected]>
> > ---
> > fs/nfsd/nfs4state.c | 5 +++++
> > 1 files changed, 5 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 582cd2c..90e6645 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -425,6 +425,11 @@ static int set_forechannel_maxreqs(struct nfsd4_channel_attrs *fchan)
> > {
> > int status = 0, np = fchan->maxreqs * NFSD_PAGES_PER_SLOT;
> >
> > + if (fchan->maxreqs < 1)
> > + return nfserr_inval;
> > + else if (fchan->maxreqs > NFSD_MAX_SLOTS_PER_SESSION)
> > + fchan->maxreqs = NFSD_MAX_SLOTS_PER_SESSION;
> > +
> > spin_lock(&nfsd_serv->sv_lock);
>
> How did this use of sv_lock creep in? Let's remove that before we do
> anything else....

(But--applied.)

--b.

>
> --b.
>
> > if (np + nfsd_serv->sv_drc_pages_used > nfsd_serv->sv_drc_max_pages)
> > np = nfsd_serv->sv_drc_max_pages - nfsd_serv->sv_drc_pages_used;
> > --
> > 1.6.3
> >

2009-06-17 01:23:12

by Andy Adamson

Subject: Re: [PATCH 09/44] nfsd41: sanity check client drc maxreqs

On Tue, Jun 16, 2009 at 8:35 PM, J. Bruce Fields<[email protected]> wrote:
> On Tue, Jun 16, 2009 at 04:19:38AM +0300, Benny Halevy wrote:
>> From: Andy Adamson <[email protected]>
>>
>> Ensure the client requested maximum requests are between 1 and
>> NFSD_MAX_SLOTS_PER_SESSION
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>> Signed-off-by: Benny Halevy <[email protected]>
>> ---
>> fs/nfsd/nfs4state.c | 5 +++++
>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 582cd2c..90e6645 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -425,6 +425,11 @@ static int set_forechannel_maxreqs(struct nfsd4_channel_attrs *fchan)
>> {
>> int status = 0, np = fchan->maxreqs * NFSD_PAGES_PER_SLOT;
>>
>> + if (fchan->maxreqs < 1)
>> + return nfserr_inval;
>> + else if (fchan->maxreqs > NFSD_MAX_SLOTS_PER_SESSION)
>> + fchan->maxreqs = NFSD_MAX_SLOTS_PER_SESSION;
>> +
>> spin_lock(&nfsd_serv->sv_lock);
>
> How did this use of sv_lock creep in? Let's remove that before we do
> anything else....

I remove it later.....

-->Andy

>
> --b.
>
>> if (np + nfsd_serv->sv_drc_pages_used > nfsd_serv->sv_drc_max_pages)
>> np = nfsd_serv->sv_drc_max_pages - nfsd_serv->sv_drc_pages_used;
>> --
>> 1.6.3
>>
>