linux-kernel - Re: [PATCH] staging: wilc1000: fix infinite loop and out-of-bounds access

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] [day] [month] [year] [list]

Message-ID: <20180502151229.4be29ec8@ajaysk-VirtualBox>
Date:   Wed, 2 May 2018 15:12:29 +0530
From:   Ajay Singh <ajay.kathat@...rochip.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
CC:     <devel@...verdev.osuosl.org>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        <linux-wireless@...r.kernel.org>,
        <kernel-janitors@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        "Ganesh Krishna" <ganesh.krishna@...rochip.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] staging: wilc1000: fix infinite loop and out-of-bounds
 access

On Wed, 2 May 2018 11:39:36 +0300
Dan Carpenter <dan.carpenter@...cle.com> wrote:

> We're mainly discussing readability, right?
> 
> To me when people use "int" that tells me as a reader that we don't
> need to think about the type.  It's going to be a small number.
> 
> Say you have data which the user can control, then it's super
> important to focus on the data types.  We don't focus on it
> enough.  There is some kind of idea that good developers should
> just be super focused on everything all the time, but I don't think
> humans can do it.  So to me it's useful when the author tells me,
> "This an int type.  It's fine. This is not critical."
> 
> If you make request->n_ssids a u8 or u16, that isn't going to save
> any memory because the struct is padded.  You'd also need to audit
> a bunch of code to make sure that we don't overflow the u16.  If
> you wanted to overflow the int, you'd need to allocate several gigs
> of memory but kmalloc() is capped at KMALLOC_MAX_SIZE (4MB) so
> that's not possible. How many of these structs do we allocate?  Is
> it really worth optimizing the heck out of it?
> 
> There are times where want to be very deliberate with our types
> because we're dealing the large numbers, or user data or fast
> paths.  But there are other times where int is fine...
> 

As in this case, its fine to be of 'int' type.
So we can retain the current data type('int') for 'i' and 'slot_id'. 
Thank you for sharing your insights,it was very helpful.

Regards,
Ajay