diff mbox series

misc: ibmvsm: Fix wrong assignment of return code

Message ID 1533562260-11456-1-git-send-email-bryantly@linux.vnet.ibm.com (mailing list archive)
State Not Applicable
Headers show
Series misc: ibmvsm: Fix wrong assignment of return code | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied
snowpatch_ozlabs/checkpatch success Test checkpatch on branch next
snowpatch_ozlabs/build-ppc64le success Test build-ppc64le on branch next
snowpatch_ozlabs/build-ppc64be success Test build-ppc64be on branch next
snowpatch_ozlabs/build-ppc64e success Test build-ppc64e on branch next
snowpatch_ozlabs/build-ppc32 success Test build-ppc32 on branch next

Commit Message

Bryant G. Ly Aug. 6, 2018, 1:31 p.m. UTC 
From: "Bryant G. Ly" <bryantly@linux.ibm.com>

Currently the assignment is flipped and rc is always 0.

Signed-off-by: Bryant G. Ly <bryantly@linux.ibm.com>
Reviewed-by: Bradley Warrum <bwarrum@us.ibm.com>
---
 drivers/misc/ibmvmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Ellerman Aug. 7, 2018, 12:28 p.m. UTC | #1 
"Bryant G. Ly" <bryantly@linux.vnet.ibm.com> writes:

> From: "Bryant G. Ly" <bryantly@linux.ibm.com>
>
> Currently the assignment is flipped and rc is always 0.

If you'd left rc uninitialised at the start of the function the compiler
would have caught it for you.

And what is the consequence of the bug? Nothing, complete system crash,
subtle data corruption?

Also this should be tagged:

Fixes: 0eca353e7ae7 ("misc: IBM Virtual Management Channel Driver (VMC)")

cheers

> diff --git a/drivers/misc/ibmvmc.c b/drivers/misc/ibmvmc.c
> index 8f82bb9..b8aaa68 100644
> --- a/drivers/misc/ibmvmc.c
> +++ b/drivers/misc/ibmvmc.c
> @@ -2131,7 +2131,7 @@ static int ibmvmc_init_crq_queue(struct crq_server_adapter *adapter)
>  	retrc = plpar_hcall_norets(H_REG_CRQ,
>  				   vdev->unit_address,
>  				   queue->msg_token, PAGE_SIZE);
> -	retrc = rc;
> +	rc = retrc;
>  
>  	if (rc == H_RESOURCE)
>  		rc = ibmvmc_reset_crq_queue(adapter);
Bryant G. Ly Aug. 7, 2018, 2:22 p.m. UTC | #2 
On 8/7/18 7:28 AM, Michael Ellerman wrote:

> "Bryant G. Ly" <bryantly@linux.vnet.ibm.com> writes:
>
>> From: "Bryant G. Ly" <bryantly@linux.ibm.com>
>>
>> Currently the assignment is flipped and rc is always 0.
> If you'd left rc uninitialised at the start of the function the compiler
> would have caught it for you.
>
> And what is the consequence of the bug? Nothing, complete system crash,
> subtle data corruption?

The consequence would be that if the CRQ Registration failed the first time
due to not enough resources, it would never try to reset and try again. 

If it fails due to any other error then it would just fail the sending of the
crq init message, thus it would just wait for the client to init, which would
never happen. 

We would also have a memory leak since in the error case DMA would never get
un-mapped and the message queue never gets freed. 

>
> Also this should be tagged:
>
> Fixes: 0eca353e7ae7 ("misc: IBM Virtual Management Channel Driver (VMC)")
>
> cheers
>
Yep, sorry I forgot to add the Fixes:..

-Bryant
diff mbox series

Patch

diff --git a/drivers/misc/ibmvmc.c b/drivers/misc/ibmvmc.c
index 8f82bb9..b8aaa68 100644
--- a/drivers/misc/ibmvmc.c
+++ b/drivers/misc/ibmvmc.c
@@ -2131,7 +2131,7 @@  static int ibmvmc_init_crq_queue(struct crq_server_adapter *adapter)
 	retrc = plpar_hcall_norets(H_REG_CRQ,
 				   vdev->unit_address,
 				   queue->msg_token, PAGE_SIZE);
-	retrc = rc;
+	rc = retrc;
 
 	if (rc == H_RESOURCE)
 		rc = ibmvmc_reset_crq_queue(adapter);