[I fixed Dan's address - sorry about that]
On Thu, 26 Sep 2024, Chuck Lever wrote:
> Hi Neil -
>
> On Wed, Sep 25, 2024 at 05:28:09PM +1000, NeilBrown wrote:
> >
> > If the rq_prog is not in the list of programs, then we use the last
> > program in the list and subsequent tests on 'progp' being NULL are
> > useless.
>
> That's the logic error, but what is the observed unexpected
> behavior?
The unexpected behaviour is that "if rq_prog is not in the list of
programs, then we use the last program in the list". Isn't that a
behaviour? Should I add that "we don't get the expected
rpc_prog_unavail error?
What am I missing?
>
>
> > We should only assign progp when we find the right program, and we
> > should initialize it to NULL
> >
> > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > Fixes: 86ab08beb3f0 ("SUNRPC: replace program list with program array")
> > Signed-off-by: NeilBrown <neilb@xxxxxxx>
>
> IIRC commit 86ab08beb3f0 went through Anna's tree during this merge
> window. It would be easier (for me) if Anna took this one.
I don't entirely understand the logic, but ok.
>
> Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
Thanks,
NeilBrown
>
>
> > ---
> > net/sunrpc/svc.c | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> > index 7e7f4e0390c7..79879b7d39cb 100644
> > --- a/net/sunrpc/svc.c
> > +++ b/net/sunrpc/svc.c
> > @@ -1321,7 +1321,7 @@ static int
> > svc_process_common(struct svc_rqst *rqstp)
> > {
> > struct xdr_stream *xdr = &rqstp->rq_res_stream;
> > - struct svc_program *progp;
> > + struct svc_program *progp = NULL;
> > const struct svc_procedure *procp = NULL;
> > struct svc_serv *serv = rqstp->rq_server;
> > struct svc_process_info process;
> > @@ -1351,12 +1351,9 @@ svc_process_common(struct svc_rqst *rqstp)
> > rqstp->rq_vers = be32_to_cpup(p++);
> > rqstp->rq_proc = be32_to_cpup(p);
> >
> > - for (pr = 0; pr < serv->sv_nprogs; pr++) {
> > - progp = &serv->sv_programs[pr];
> > -
> > - if (rqstp->rq_prog == progp->pg_prog)
> > - break;
> > - }
> > + for (pr = 0; pr < serv->sv_nprogs; pr++)
> > + if (rqstp->rq_prog == serv->sv_programs[pr].pg_prog)
> > + progp = &serv->sv_programs[pr];
> >
> > /*
> > * Decode auth data, and add verifier to reply buffer.
> > --
> > 2.46.0
> >
>
> --
> Chuck Lever
>
