linux-kernel - Re: Change in register

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:   Thu, 1 Feb 2018 18:17:31 -0800
From:   "Srivatsa S. Bhat" <srivatsa@...il.mit.edu>
To:     Logan Gunthorpe <logang@...tatee.com>, gregkh@...uxfoundation.org
Cc:     axboe@...nel.dk, jlayton@...chiereds.net, bfields@...ldses.org,
        linux-kernel@...r.kernel.org
Subject: Re: Change in register_blkdev() behavior

On 2/1/18 5:10 PM, Logan Gunthorpe wrote:
> 
> 
> On 01/02/18 05:23 PM, Srivatsa S. Bhat wrote:
>> static int find_dynamic_major(void)
>> {
>>          int i;
>>          struct char_device_struct *cd;
>>
>>          for (i = ARRAY_SIZE(chrdevs)-1; i > CHRDEV_MAJOR_DYN_END; i--) {
>>                                           ^^^^
>> As far as I can see, _DYN_END is inclusive, so shouldn't this be >= ?
> 
> Yes, it looks like _DYN_END should have been inclusive based on the way I documented it.
> 

Thank you for confirming! I'll send a patch to fix that (and the analogous
case for CHRDEV_MAJOR_DYN_EXT_END).

>>
>>                  for (cd = chrdevs[major_to_index(i)]; cd; cd = cd->next)
>>                          if (cd->major == i)
>>                                  break;
>>
>>                  if (cd == NULL || cd->major != i)
>>                                       ^^^^^^^^
>> It seems that this latter condition is unnecessary, as it will never be
>> true. (We'll reach here only if cd == NULL or cd->major == i).
> 
> Not quite. chrdevs[] may contain majors that also hit on the hash but don't equal 'i'. So the for loop will iterate through all hashes matching 'i' and if there is one or more and they all don't match 'i', it will fall through the loop and cd will be set to something non-null and not equal to i.
> 

Hmm, the code doesn't appear to be doing that though? The loop's fall
through occurs one past the last entry, when cd == NULL. The only
other way it can exit the loop is if it hits the break statement
(which implies that cd->major == i). So what am I missing?

Regards,
Srivatsa