lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 27 Jun 2008 12:22:32 +0300
From:	Benny Halevy <bhalevy@...asas.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
CC:	Greg KH <greg@...ah.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: fix uninitialized variables in keyspan_pda

On Jun. 27, 2008, 11:58 +0300, Alan Cox <alan@...rguk.ukuu.org.uk> wrote:
> On Fri, 27 Jun 2008 10:58:54 +0300
> Benny Halevy <bhalevy@...asas.com> wrote:
> 
>> The compiler (gcc 4.3.0) warns that status might be used uninitialized
>> in keyspan_pda_tiocmget and keyspan_pda_tiocmset.
>>
>> It is technically correct and therefore this patch initializes
>> status to 0 in both cases.
>>
>> Note that keyspan_pda_get_modem_info sets *value only when
>> usb_control_msg returns rc > 0, otherwise it must
>> return an error rc < 0, and it must never return rc == 0, hence
>> a WARN_ON(rc == 0) was added in case usb_control_msg ever return 0.
> 
> That seems like overkill - and by forcing status to 0 you will hide
> future gcc error catches. Far better to change the two callers to test
> for > 0 not >= 0 surely ?

Well, since usb_control_msg is not supposed to ever return 0
I'm really not sure what to do if it does. I'll gladly defer that
decision to Greg...

The following patch makes gcc happy as you said and if it's
correct I, too, like it much better.

Benny

diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c
index ff54203..197d283 100644
--- a/drivers/usb/serial/keyspan_pda.c
+++ b/drivers/usb/serial/keyspan_pda.c
@@ -420,7 +420,7 @@ static int keyspan_pda_get_modem_info(struct usb_serial *serial,
                             3, /* get pins */
                             USB_TYPE_VENDOR|USB_RECIP_INTERFACE|USB_DIR_IN,
                             0, 0, &data, 1, 2000);
-       if (rc > 0)
+       if (rc >= 0)
                *value = data;
        return rc;
 }

> 
> Alan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ