[pnfs] [PATCH 1/8] nfsd: return nfserr_minor_vers_mismatch when compound minorversion != 0

[J. Bruce Fields]

On Tue, Jul 01, 2008 at 04:44:42PM +0300, Benny Halevy wrote:
> On Jun. 30, 2008, 23:06 +0300, Benny Halevy <bhalevy at panasas.com> wrote:
> > J. Bruce Fields wrote:
> >> On Mon, Jun 30, 2008 at 09:03:59PM +0300, Benny Halevy wrote:
> >>> Check minorversion once before decoding any operation and reject with
> >>> nfserr_minor_vers_mismatch if != 0.  In this case return a zero length
> >>> resultdata array as required by RFC3530.
> >> I think there's no actual change in behavior here, right?  On a quick
> >> skim, it looks to me like the check in nfsd4_proc_compound() does the
> >> right thing.
> > 
> > Right.
> > 
> >> That's nothing against this patch, though maybe it'd make sense to
> >> delete the minorversion check from nfsd4_proc_compound().
> > 
> > Agreed.
> 
> I tried that and unfortunately, I have to take my words back.
> We still need to go through nfsd4_proc_compound to encode the
> reply (containing the header only and no ops)
> 
> Here's the updated patch:
> (no need to set up anything for the first op since it's
> not processed)

OK.  Well, the result's a simple enough patch!  Thanks.  Could you
resend with an updated changelog?

Also--do we have a newpynfs test for this?  If not, it might be nice to
add one some day (along with any other obvious checks of basic
minorversion field handling), possibly with Fred's help.

--b.

> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 9547ab6..413a153 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1019,6 +1019,9 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
>  		}
>  	}
>  
> +	if (argp->minorversion != 0)
> +		argp->opcnt = 0;
> +
>  	for (i = 0; i < argp->opcnt; i++) {
>  		op = &argp->ops[i];
>  		op->replay = NULL;
> @@ -1057,13 +1060,6 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
>  		op->opnum = ntohl(*argp->p++);
>  
>  		switch (op->opnum) {
> -		case 2: /* Reserved operation */
> -			op->opnum = OP_ILLEGAL;
> -			if (argp->minorversion == 0)
> -				op->status = nfserr_op_illegal;
> -			else
> -				op->status = nfserr_minor_vers_mismatch;
> -			break;
>  		case OP_ACCESS:
>  			op->status = nfsd4_decode_access(argp, &op->u.access);
>  			break;
> 
> 
> > 
> > Benny
> > 
> >> --b.
> >>
> >>> minorversion 1 processing will have its own vector of decoders.
> >>>
> >>> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> >>> ---
> >>>  fs/nfsd/nfs4xdr.c |   15 ++++++++-------
> >>>  1 files changed, 8 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> >>> index 9547ab6..491a697 100644
> >>> --- a/fs/nfsd/nfs4xdr.c
> >>> +++ b/fs/nfsd/nfs4xdr.c
> >>> @@ -1019,6 +1019,14 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
> >>>  		}
> >>>  	}
> >>>  
> >>> +	if (argp->minorversion != 0) {
> >>> +		op = &argp->ops[0];
> >>> +		op->replay = NULL;
> >>> +		op->opnum = OP_ILLEGAL;
> >>> +		op->status = nfserr_minor_vers_mismatch;
> >>> +		argp->opcnt = 0;
> >>> +	}
> >>> +
> >>>  	for (i = 0; i < argp->opcnt; i++) {
> >>>  		op = &argp->ops[i];
> >>>  		op->replay = NULL;
> >>> @@ -1057,13 +1065,6 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
> >>>  		op->opnum = ntohl(*argp->p++);
> >>>  
> >>>  		switch (op->opnum) {
> >>> -		case 2: /* Reserved operation */
> >>> -			op->opnum = OP_ILLEGAL;
> >>> -			if (argp->minorversion == 0)
> >>> -				op->status = nfserr_op_illegal;
> >>> -			else
> >>> -				op->status = nfserr_minor_vers_mismatch;
> >>> -			break;
> >>>  		case OP_ACCESS:
> >>>  			op->status = nfsd4_decode_access(argp, &op->u.access);
> >>>  			break;
> >>> -- 
> >>> 1.5.6.GIT
> >>>
> > 
> > _______________________________________________
> > pNFS mailing list
> > pNFS at linux-nfs.org
> > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>