diff mbox series

sunrpc: fix prog selection loop in svc_process_common

Message ID 172741974136.470955.11402099885897272884@noble.neil.brown.name (mailing list archive)
State New
Headers show
Series sunrpc: fix prog selection loop in svc_process_common | expand

Commit Message

NeilBrown Sept. 27, 2024, 6:49 a.m. UTC
If the rq_prog is not in the list of programs, then we use the last
program in the list and we don't get the expected rpc_prog_unavail error
as the subsequent tests on 'progp' being NULL are ineffective.

We should only assign progp when we find the right program, and we
should initialize it to NULL

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Fixes: 86ab08beb3f0 ("SUNRPC: replace program list with program array")
Signed-off-by: NeilBrown <neilb@suse.de>
---

Hi Anna,
 could you take this please - a fix to a patch in your latest pull
 request to Linus.
Thanks,
NeilBrown


 net/sunrpc/svc.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Chuck Lever Sept. 27, 2024, 2:19 p.m. UTC | #1
On Fri, Sep 27, 2024 at 04:49:01PM +1000, NeilBrown wrote:
> 
> If the rq_prog is not in the list of programs, then we use the last
> program in the list and we don't get the expected rpc_prog_unavail error
> as the subsequent tests on 'progp' being NULL are ineffective.
> 
> We should only assign progp when we find the right program, and we
> should initialize it to NULL
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Fixes: 86ab08beb3f0 ("SUNRPC: replace program list with program array")
> Signed-off-by: NeilBrown <neilb@suse.de>

Acked-by: Chuck Lever <chuck.lever@oracle.com>


> ---
> 
> Hi Anna,
>  could you take this please - a fix to a patch in your latest pull
>  request to Linus.
> 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
>
Anna Schumaker Sept. 30, 2024, 7:38 p.m. UTC | #2
On 9/27/24 10:19 AM, Chuck Lever wrote:
> On Fri, Sep 27, 2024 at 04:49:01PM +1000, NeilBrown wrote:
>>
>> If the rq_prog is not in the list of programs, then we use the last
>> program in the list and we don't get the expected rpc_prog_unavail error
>> as the subsequent tests on 'progp' being NULL are ineffective.
>>
>> We should only assign progp when we find the right program, and we
>> should initialize it to NULL
>>
>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>> Fixes: 86ab08beb3f0 ("SUNRPC: replace program list with program array")
>> Signed-off-by: NeilBrown <neilb@suse.de>
> 
> Acked-by: Chuck Lever <chuck.lever@oracle.com>
> 
> 
>> ---
>>
>> Hi Anna,
>>  could you take this please - a fix to a patch in your latest pull
>>  request to Linus.
>> Thanks,
>> NeilBrown
>>

Sure! I'll get it queued up, and push out bugfixes to my linux-next branch in the next day or two.

Anna

>>
>>  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
>>
>
diff mbox series

Patch

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.