From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8D917C73C73 for ; Wed, 10 Jul 2019 05:49:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6F21D20693 for ; Wed, 10 Jul 2019 05:49:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727143AbfGJFtm (ORCPT ); Wed, 10 Jul 2019 01:49:42 -0400 Received: from relay.sw.ru ([185.231.240.75]:49196 "EHLO relay.sw.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725844AbfGJFtl (ORCPT ); Wed, 10 Jul 2019 01:49:41 -0400 Received: from [172.16.24.21] by relay.sw.ru with esmtp (Exim 4.92) (envelope-from ) id 1hl5Ti-0002oO-18; Wed, 10 Jul 2019 08:49:18 +0300 Subject: Re: [PATCH v3 0/3] kernel/notifier.c: avoid duplicate registration To: Xiaoming Ni , adobriyan@gmail.com, akpm@linux-foundation.org, anna.schumaker@netapp.com, arjan@linux.intel.com, bfields@fieldses.org, chuck.lever@oracle.com, davem@davemloft.net, gregkh@linuxfoundation.org, jlayton@kernel.org, luto@kernel.org, mingo@kernel.org, Nadia.Derbey@bull.net, paulmck@linux.vnet.ibm.com, semen.protsenko@linaro.org, stable@kernel.org, stern@rowland.harvard.edu, tglx@linutronix.de, torvalds@linux-foundation.org, trond.myklebust@hammerspace.com, viresh.kumar@linaro.org Cc: alex.huangjianhui@huawei.com, dylix.dailei@huawei.com, linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org, netdev@vger.kernel.org References: <1562728147-30251-1-git-send-email-nixiaoming@huawei.com> From: Vasily Averin Message-ID: Date: Wed, 10 Jul 2019 08:49:06 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <1562728147-30251-1-git-send-email-nixiaoming@huawei.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/10/19 6:09 AM, Xiaoming Ni wrote: > Registering the same notifier to a hook repeatedly can cause the hook > list to form a ring or lose other members of the list. I think is not enough to _prevent_ 2nd register attempt, it's enough to detect just attempt and generate warning to mark host in bad state. Unexpected 2nd register of the same hook most likely will lead to 2nd unregister, and it can lead to host crash in any time: you can unregister notifier on first attempt it can be too early, it can be still in use. on the other hand you can never call 2nd unregister at all. Unfortunately I do not see any ways to handle such cases properly, and it seems for me your patches does not resolve this problem. Am I missed something probably? > case1: An infinite loop in notifier_chain_register() can cause soft lockup > atomic_notifier_chain_register(&test_notifier_list, &test1); > atomic_notifier_chain_register(&test_notifier_list, &test1); > atomic_notifier_chain_register(&test_notifier_list, &test2); > > case2: An infinite loop in notifier_chain_register() can cause soft lockup > atomic_notifier_chain_register(&test_notifier_list, &test1); > atomic_notifier_chain_register(&test_notifier_list, &test1); > atomic_notifier_call_chain(&test_notifier_list, 0, NULL); > > case3: lose other hook test2 > atomic_notifier_chain_register(&test_notifier_list, &test1); > atomic_notifier_chain_register(&test_notifier_list, &test2); > atomic_notifier_chain_register(&test_notifier_list, &test1); > > case4: Unregister returns 0, but the hook is still in the linked list, > and it is not really registered. If you call notifier_call_chain > after ko is unloaded, it will trigger oops. if the system is > configured with softlockup_panic and the same hook is repeatedly > registered on the panic_notifier_list, it will cause a loop panic. > > so. need add a check in in notifier_chain_register() to avoid duplicate > registration > > v1: > * use notifier_chain_cond_register replace notifier_chain_register > > v2: > * Add a check in notifier_chain_register() to avoid duplicate registration > * remove notifier_chain_cond_register() to avoid duplicate code > * remove blocking_notifier_chain_cond_register() to avoid duplicate code > > v3: > * Add a cover letter. > > Xiaoming Ni (3): > kernel/notifier.c: avoid duplicate registration > kernel/notifier.c: remove notifier_chain_cond_register() > kernel/notifier.c: remove blocking_notifier_chain_cond_register() > > include/linux/notifier.h | 4 ---- > kernel/notifier.c | 41 +++-------------------------------------- > net/sunrpc/rpc_pipe.c | 2 +- > 3 files changed, 4 insertions(+), 43 deletions(-) >