[PATCH V3] staging: gpib: avoid unintended sign extension

[PATCH V3] staging: gpib: avoid unintended sign extension

[Kees Bakker]

The code was basically like this (assuming size_t can be u64)
    var_u64 |= var_u8 << 24
var_u8 is first promoted to i32 and then the shift is done. Next, it is
promoted to u64 by first signextending to 64 bits. This is very unlikely
what was intended. So now it is first forced to u32.
    var_u64 |= (u32)var_u8 << 24

This was detected by Coverity, CID 1600792.

Fixes: 4c41fe886a56 ("staging: gpib: Add Agilent/Keysight 82357x USB GPIB driver")
Signed-off-by: Kees Bakker <kees@ijzerbout.nl>
---
 V1 -> V2: Changed commit message, added Fixes: tag and a reference to the Coverity CID
 V2 -> V3: add version in the subject (sorry Greg)

 drivers/staging/gpib/agilent_82357a/agilent_82357a.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gpib/agilent_82357a/agilent_82357a.c b/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
index bbc7e8866872..f3a1b81be22d 100644
--- a/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
+++ b/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
@@ -679,10 +679,10 @@ static ssize_t agilent_82357a_generic_write(gpib_board_t *board, uint8_t *buffer
 		kfree(status_data);
 		return -EIO;
 	}
-	*bytes_written	= status_data[2];
-	*bytes_written |= status_data[3] << 8;
-	*bytes_written |= status_data[4] << 16;
-	*bytes_written |= status_data[5] << 24;
+	*bytes_written	= (u32)status_data[2];
+	*bytes_written |= (u32)status_data[3] << 8;
+	*bytes_written |= (u32)status_data[4] << 16;
+	*bytes_written |= (u32)status_data[5] << 24;
 
 	kfree(status_data);
 	//printk("%s: write completed, bytes_written=%i\n", __func__, (int)*bytes_written);
-- 
2.47.0

Re: [PATCH V3] staging: gpib: avoid unintended sign extension

[Greg KH]

On Thu, Oct 17, 2024 at 09:54:47PM +0200, Kees Bakker wrote:
> The code was basically like this (assuming size_t can be u64)
>     var_u64 |= var_u8 << 24
> var_u8 is first promoted to i32 and then the shift is done. Next, it is
> promoted to u64 by first signextending to 64 bits. This is very unlikely
> what was intended. So now it is first forced to u32.
>     var_u64 |= (u32)var_u8 << 24
> 
> This was detected by Coverity, CID 1600792.
> 
> Fixes: 4c41fe886a56 ("staging: gpib: Add Agilent/Keysight 82357x USB GPIB driver")
> Signed-off-by: Kees Bakker <kees@ijzerbout.nl>
> ---
>  V1 -> V2: Changed commit message, added Fixes: tag and a reference to the Coverity CID
>  V2 -> V3: add version in the subject (sorry Greg)

Doesn't apply cleanly anymore :(

Also, always cc: all of the relevent people that get_maintainer.pl says
to cc:.  Can you rebase this against my staging-testing branch and
resend?

thanks,

greg k-h