linux-kernel - RE: [RFC PATCH] char: misc: Init misc->list in a safe way

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]

Message-ID: <f556152beaac4a6ab91cd9029eaf00fc@SHMBX02.spreadtrum.com>
Date:   Mon, 26 Jun 2017 11:48:14 +0000
From:   Zhongping Tan (谭中平) 
        <Zhongping.Tan@...eadtrum.com>
To:     Arnd Bergmann <arnd@...db.de>,
        Orson Zhai (翟京) 
        <Orson.Zhai@...eadtrum.com>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH] char: misc: Init misc->list in a safe way

Hi Arnd：
If we can get list_add(&misc->list, &misc_list), then there is no problem at all, but if the misc_register return "-EBUSY"(Maybe the same miscdevice register twice ), then the deadloop will happen at list_for_each_entry(c, &misc_list, list); 
So at my opinion just remove the initialization code or do initialization when we need do list_add.

-----Original Message-----
From: arndbergmann@...il.com [mailto:arndbergmann@...il.com] On Behalf Of Arnd Bergmann
Sent: Monday, June 26, 2017 6:03 PM
To: Orson Zhai (翟京)
Cc: Greg Kroah-Hartman; Linux Kernel Mailing List; Zhongping Tan (谭中平)
Subject: Re: [RFC PATCH] char: misc: Init misc->list in a safe way

On Mon, Jun 26, 2017 at 11:31 AM, Orson Zhai <orson.zhai@...eadtrum.com> wrote:
> From: Zhongping Tan <zhongping.tan@...eadtrum.com>
>
> It is likely to enter a wrong case and return an error when 
> registerring a misc device. As a result, misc->list will be intialized 
> to a dead loop which is possible to go into wrong situation if anyone 
> refers to it else where.
>
> Move the initializion line out of all error branches to avoid any side 
> effect.
>
> Signed-off-by: Zhongping Tan <zhongping.tan@...eadtrum.com>
> Signed-off-by: Orson Zhai <orson.zhai@...eadtrum.com>

I fail to see the problem. Did you run into a bug that gets fixed by this patch, or did you arrive here after code inspection?

As far as I can tell, the INIT_LIST_HEAD() on the entry has no effect at all, the fields simply get initialized in the list_add(), and the list traversal is protected using misc_mtx.

       Arnd