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=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 4B606C433E1 for ; Thu, 18 Jun 2020 19:08:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 38289207E8 for ; Thu, 18 Jun 2020 19:08:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730268AbgFRTI1 (ORCPT ); Thu, 18 Jun 2020 15:08:27 -0400 Received: from nautica.notk.org ([91.121.71.147]:34180 "EHLO nautica.notk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728071AbgFRTI1 (ORCPT ); Thu, 18 Jun 2020 15:08:27 -0400 Received: by nautica.notk.org (Postfix, from userid 1001) id EA9E8C01C; Thu, 18 Jun 2020 21:08:22 +0200 (CEST) Date: Thu, 18 Jun 2020 21:08:07 +0200 From: Dominique Martinet To: Alexander Kapshuk Cc: ericvh@gmail.com, lucho@ionkov.net, davem@davemloft.net, kuba@kernel.org, v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] net/9p: Fix sparse rcu warnings in client.c Message-ID: <20200618190807.GA20699@nautica> References: <20200618183310.5352-1-alexander.kapshuk@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200618183310.5352-1-alexander.kapshuk@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Alexander Kapshuk wrote on Thu, Jun 18, 2020: > Address sparse nonderef rcu warnings: > net/9p/client.c:790:17: warning: incorrect type in argument 1 (different address spaces) > net/9p/client.c:790:17: expected struct spinlock [usertype] *lock > net/9p/client.c:790:17: got struct spinlock [noderef] * > net/9p/client.c:792:48: warning: incorrect type in argument 1 (different address spaces) > net/9p/client.c:792:48: expected struct spinlock [usertype] *lock > net/9p/client.c:792:48: got struct spinlock [noderef] * > net/9p/client.c:872:17: warning: incorrect type in argument 1 (different address spaces) > net/9p/client.c:872:17: expected struct spinlock [usertype] *lock > net/9p/client.c:872:17: got struct spinlock [noderef] * > net/9p/client.c:874:48: warning: incorrect type in argument 1 (different address spaces) > net/9p/client.c:874:48: expected struct spinlock [usertype] *lock > net/9p/client.c:874:48: got struct spinlock [noderef] * > > Signed-off-by: Alexander Kapshuk Thanks for this patch. >From what I can see, there are tons of other parts of the code doing the same noderef access pattern to access current->sighand->siglock and I don't see much doing that. A couple of users justify this by saying SLAB_TYPESAFE_BY_RCU ensures we'll always get a usable lock which won't be reinitialized however we access it... It's a bit dubious we'll get the same lock than unlock to me, so I agree to some change though. After a second look I think we should use something like the following: if (!lock_task_sighand(current, &flags)) warn & skip (or some error, we'd null deref if this happened currently); recalc_sigpending(); unlock_task_sighand(current, &flags); As you can see, the rcu_read_lock() isn't kept until the unlock so I'm not sure it will be enough to please sparse, but I've convinced myself current->sighand cannot change while we hold the lock and there just are too many such patterns in the kernel. Please let me know if I missed something or if there is an ongoing effort to change how this works; I'll wait for a v2. -- Dominique