linux-kernel - Re: [patch 1/2] mm, mmu_notifier: annotate mmu notifiers with blockable invalidate callbacks

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:   Wed, 13 Dec 2017 11:37:25 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        Christian König <christian.koenig@....com>,
        David Rientjes <rientjes@...gle.com>,
        Dimitri Sivanich <sivanich@....com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Oded Gabbay <oded.gabbay@...il.com>,
        Alex Deucher <alexander.deucher@....com>,
        David Airlie <airlied@...ux.ie>,
        Joerg Roedel <joro@...tes.org>,
        Doug Ledford <dledford@...hat.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Mike Marciniszyn <mike.marciniszyn@...el.com>,
        Sean Hefty <sean.hefty@...el.com>,
        Dimitri Sivanich <sivanich@....com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Jérôme Glisse <jglisse@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>
Subject: Re: [patch 1/2] mm, mmu_notifier: annotate mmu notifiers with
 blockable invalidate callbacks

On 13/12/2017 11:26, Tetsuo Handa wrote:
> On 2017/12/13 18:34, Christian König wrote:
>> Am 12.12.2017 um 22:28 schrieb David Rientjes:
>>> On Tue, 12 Dec 2017, Dimitri Sivanich wrote:
>>>
>>>>> --- a/drivers/misc/sgi-gru/grutlbpurge.c
>>>>> +++ b/drivers/misc/sgi-gru/grutlbpurge.c
>>>>> @@ -298,6 +298,7 @@ struct gru_mm_struct *gru_register_mmu_notifier(void)
>>>>>               return ERR_PTR(-ENOMEM);
>>>>>           STAT(gms_alloc);
>>>>>           spin_lock_init(&gms->ms_asid_lock);
>>>>> +        gms->ms_notifier.flags = 0;
>>>>>           gms->ms_notifier.ops = &gru_mmuops;
>>>>>           atomic_set(&gms->ms_refcnt, 1);
>>>>>           init_waitqueue_head(&gms->ms_wait_queue);
>>>>> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
>>>> There is a kzalloc() just above this:
>>>>     gms = kzalloc(sizeof(*gms), GFP_KERNEL);
>>>>
>>>> Is that not sufficient to clear the 'flags' field?
>>>>
>>> Absolutely, but whether it is better to explicitly document that the mmu
>>> notifier has cleared flags, i.e. there are no blockable callbacks, is
>>> another story.  I can change it if preferred.
>>
>> Actually I would invert the new flag, in other words specify that an MMU notifier will never sleep.
>>
>> The first reason is that we have 8 blocking notifiers and 5 not blocking if I counted right. So it is actually more common to sleep than not to.
>>
>> The second reason is to be conservative and assume the worst, e.g. that the flag is forgotten when a new notifier is added.
> 
> I agree. Some out of tree module might forget to set the flags.
> 
> Although you don't need to fix out of tree modules, as a troubleshooting
> staff at a support center, I want to be able to identify the careless module.
> 
> I guess specifying the flags at register function would be the best, for
> an attempt to call register function without knowing this change will
> simply results in a build failure.

Specifying them in the ops would have the same effect and it would be
even better, as you don't have to split the information across two places.

Paolo