linux-kernel - Re: [PATCH v2] debugobjects: Move printk out of db lock critical sections

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: <e1e313d9-429d-fb9a-8ebe-74b43eda077e@redhat.com>
Date:   Mon, 17 Dec 2018 13:33:07 -0500
From:   Waiman Long <longman@...hat.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Yang Shi <yang.shi@...ux.alibaba.com>,
        Arnd Bergmann <arnd@...db.de>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Dmitry Safonov <dima@...sta.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v2] debugobjects: Move printk out of db lock critical
 sections

On 12/17/2018 01:17 PM, Ingo Molnar wrote:
> * Waiman Long <longman@...hat.com> wrote:
>
>> The db->lock is a raw spinlock and so the lock hold time is supposed to
>> be short. This will not be the case when printk() is being involved in
>> some of the critical sections.
>>
>> In order to avoid the long hold time, in case some messages need to be
>> printed, all the debug_object_is_on_stack() and debug_print_object()
>> calls are now moved out of those critical sections in the following
>> functions.
>>
>>  - __debug_object_init()
>>  - debug_object_activate()
>>  - debug_object_deactivate()
>>  - debug_object_destroy()
>>  - debug_object_free()
>>  - debug_object_active_state()
>>  - __debug_check_no_obj_freed()
>>  - check_results()
>>
>> Holding the db->lock while calling printk() may lead to deadlock if
>> printk() somehow requires the allocation/freeing of debug object that
>> happens to be in the same hash bucket or a circular lock dependency
>> warning from lockdep as reported in
>>
>>   https://lkml.kernel.org/r/20181211091154.GL23332@shao2-debian
> This makes me sad - whatever happened to the principle of keeping printk 
> simple?
>
> We should rename printk() to syslog() or so, and rename early_printk() to 
> printk(), and be done with this.
>
> Thanks,
>
> 	Ingo

The circular lock dependency actually happened because of the serial
console code that was called by printk() to display the message. We have
multiple console drivers that may be called depending on the hardware.
It can be hard to make sure that none of them will allocate or free
objects during the call.

Cheers,
Longman