Re: Fwd: New Defects reported by Coverity Scan for Linux

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sat, Apr 12, 2014 at 07:29:29AM -0600, Bjorn Helgaas wrote:
> FYI, looks like these were added by a4dff76924fe ("x86/gpu: Add Intel
> graphics stolen memory quirk for gen2 platforms").

Some of the affected gen2 platforms do support up to 2GB of RAM which
means that if the sign extension were to happen they could hit this.
However I believe all gen2 platforms are 32bit which AFAIK makes size_t 
32 bits. So looks like we can't hit this in practice..

But if someone were to change the return type to 64bits we'd
be in real danger, so I guess it would be better to fix the bug
anyway.

-#define KB(x)  ((x) * 1024)
+#define KB(x)  ((x) * 1024U)
should be sufficient to eliminate the problem. If someone wants me to
put that into a real patch and send it out let me know.

> 
> 
> ---------- Forwarded message ----------
> From:  <scan-admin@coverity.com>
> Date: Sat, Apr 12, 2014 at 1:24 AM
> Subject: New Defects reported by Coverity Scan for Linux
> To:
> 
> ...
> 
> ** CID 1201423:  Unintended sign extension  (SIGN_EXTENSION)
> /arch/x86/kernel/early-quirks.c: 290 in i830_mem_size()
> 
> ** CID 1201424:  Unintended sign extension  (SIGN_EXTENSION)
> /arch/x86/kernel/early-quirks.c: 295 in i85x_mem_size()
> 
> ...
> ________________________________________________________________________________________________________
> *** CID 1201423:  Unintended sign extension  (SIGN_EXTENSION)
> /arch/x86/kernel/early-quirks.c: 290 in i830_mem_size()
> 284
> 285             return MB(1);
> 286     }
> 287
> 288     static size_t __init i830_mem_size(void)
> 289     {
> >>>     CID 1201423:  Unintended sign extension  (SIGN_EXTENSION)
> >>>     Suspicious implicit sign extension: "read_pci_config_byte(0, 0, 0, 99)" with type "unsigned char" (8 bits, unsigned) is promoted in "read_pci_config_byte(0, 0, 0, 99) * 33554432" to type "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits, unsigned).  If "read_pci_config_byte(0, 0, 0, 99) * 33554432" is greater than 0x7FFFFFFF, the upper bits of the result will all be 1.
> 290             return read_pci_config_byte(0, 0, 0, I830_DRB3) * MB(32);
> 291     }
> 292
> 293     static size_t __init i85x_mem_size(void)
> 294     {
> 295             return read_pci_config_byte(0, 0, 1, I85X_DRB3) * MB(32);
> 
> ________________________________________________________________________________________________________
> *** CID 1201424:  Unintended sign extension  (SIGN_EXTENSION)
> /arch/x86/kernel/early-quirks.c: 295 in i85x_mem_size()
> 289     {
> 290             return read_pci_config_byte(0, 0, 0, I830_DRB3) * MB(32);
> 291     }
> 292
> 293     static size_t __init i85x_mem_size(void)
> 294     {
> >>>     CID 1201424:  Unintended sign extension  (SIGN_EXTENSION)
> >>>     Suspicious implicit sign extension: "read_pci_config_byte(0, 0, 1, 67)" with type "unsigned char" (8 bits, unsigned) is promoted in "read_pci_config_byte(0, 0, 1, 67) * 33554432" to type "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits, unsigned).  If "read_pci_config_byte(0, 0, 1, 67) * 33554432" is greater than 0x7FFFFFFF, the upper bits of the result will all be 1.
> 295             return read_pci_config_byte(0, 0, 1, I85X_DRB3) * MB(32);
> 296     }
> 297
> 298     /*
> 299      * On 830/845/85x the stolen memory base isn't available in any
> 300      * register. We need to calculate it as TOM-TSEG_SIZE-stolen_size.

-- 
Ville Syrjälä
Intel OTC