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?
> 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.
Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> 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
