Skip to contentSkip to conversation

serial: 8250_pci: Fix uninitialized variable use in pci_oxsemi_tornado_get_divisor

PATCH In review Lee Yongjun 2 participants 3 messages Jan 21 – Jan 22 TTY layer and serial drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 serial: 8250_pci: Fix uninitialized variable use in pci_oxsemi_tornado_get_divisor

Smatch reports the following errors:

drivers/tty/serial/8250/8250_pci.c:1246 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'tcr'. drivers/tty/serial/8250/8250_pci.c:1247 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'quot'. drivers/tty/serial/8250/8250_pci.c:1251 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'quot'. drivers/tty/serial/8250/8250_pci.c:1255 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'cpr'. drivers/tty/serial/8250/8250_pci.c:1265 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'cpr'.

These variables are initialized in the if branch or inside the loop in the else branch. However, if the loop finishes without finding a suitable divisor, they remain uninitialized.

Initialize 'quot', 'cpr', and 'tcr' to 0 to prevent undefined behavior.

---
 drivers/tty/serial/8250/8250_pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/tty/serial/8250/8250_pci.c +3 -3
index c5a932f48f74..b32d7e03c29a 100644
@@ -1200,9 +1200,9 @@ static unsigned int pci_oxsemi_tornado_get_divisor(struct uart_port *port,
 	unsigned int sdiv = DIV_ROUND_CLOSEST(sclk, baud);
 	unsigned int best_squot;
 	unsigned int squot;
-	unsigned int quot;
-	u16 cpr;
-	u8 tcr;
+	unsigned int quot = 0;
+	u16 cpr = 0;
+	u8 tcr = 0;
 	int i;
 
 	/* Old custom speed handling.  */
Signature · 1 line
2.34.1
#1 lore raw
Quoted 11 lines · Lee Yongjun

Smatch reports the following errors:

drivers/tty/serial/8250/8250_pci.c:1246 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'tcr'. drivers/tty/serial/8250/8250_pci.c:1247 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'quot'. drivers/tty/serial/8250/8250_pci.c:1251 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'quot'. drivers/tty/serial/8250/8250_pci.c:1255 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'cpr'. drivers/tty/serial/8250/8250_pci.c:1265 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'cpr'.

These variables are initialized in the if branch or inside the loop in the else branch. However, if the loop finishes without finding a suitable divisor, they remain uninitialized.

But can that ever really happen? Please step through it all with the values that the hardware provides to see this.

Quoted 1 line

Initialize 'quot', 'cpr', and 'tcr' to 0 to prevent undefined behavior.

Why haven't we seen any real problems with this? I think this is ok as-is. Maybe a little bit complex and non-intutive, but ok.

thanks,

greg k-h

#2 lore raw

Hi Greg,

Thanks for the clarification.

I agree that this case cannot occur in practice and the current code is fine as-is. I will drop this patch.

Thanks,

Lee Yongjun

Hi Greg,

Thanks for the clarification.

I agree that this case cannot occur in practice and the current code is fine as-is. I will drop this patch.

Thanks,

Lee Yongjun