Received: by 2002:a6b:fb09:0:0:0:0:0 with SMTP id h9csp4573571iog; Wed, 22 Jun 2022 01:19:39 -0700 (PDT) X-Google-Smtp-Source: AGRyM1ufYCkkEmvKt+PaCFlK3Keiemj6MeB7Vv4b88+uWQPATnQo7iNhCOo8KQbpYHimyUj9gIcL X-Received: by 2002:a50:ef12:0:b0:435:81f3:a93a with SMTP id m18-20020a50ef12000000b0043581f3a93amr2722733eds.80.1655885979224; Wed, 22 Jun 2022 01:19:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1655885979; cv=none; d=google.com; s=arc-20160816; b=afrQoXe39gxooiSEpDOZXUAElmBu31RrhvP4VoFqCfLv8PnIXHYT+X48HtQQfRHyYu nfa2zC0M+PMwHv130rawAeoCPkPPNZ2u8mP4O3lnEwBGWr79P+Ig9sSQWq3z7vOjShYo tdHWpCFMWTDPYMLy6E7jw8mfzVKTgKhbcbAlVtuljnOjci4ekHJ8UNicYZVpLFUMRqi7 Yv4UmmuGY92wnQmokM7PioE71LnbzH+jUTubs9JQ0qT8bvFsT9fabZshZvWqz+yonYXd 3UbFE3PIYCaqUHvy0t4nO4LTa035QLPiMvhBowofGR4cDKH3jZEvzF6eD6L6z1w1sQbi wE4A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-language:content-transfer-encoding :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:dkim-signature; bh=kZTnS3VTllrLoaJqF/w8mR2VHKq3Lk7ph/8GKtgAs1s=; b=slS9zejeTOB8lPadIUBQAzhcvEcpC1JvYOWbL0+Q7w94aMSKKqwCiHMoBRiPO8i+Pm nJ8gWPIVmx1mnODGpR5hhvD8ShlnfyoMlQYjMzcmyRnse+3bR4gbmxbWYOy1nXJVhKVo BKSwTw8Lui4QfEG6spknv5K/QVyBPGftfGm3K21oHW3FOR7CKhhRbUJSq+2nJexS29vi Q102LkRJbr2aiM5ZwdC4/yWsbS4dLhyN7+mqzcPG8aCMSFTyDZZg1HMeGxicMyzKBvQP VX7xgk8sUvFthassUDhQmLLGdE//XxxCVuhYtIw9SpKd6xK+k71EHv2HUjxpKnQxgeNi HMww== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=kwtFYGvf; spf=pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-crypto-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id dn22-20020a17090794d600b006fe94f688d8si16901455ejc.928.2022.06.22.01.19.03; Wed, 22 Jun 2022 01:19:39 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=kwtFYGvf; spf=pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-crypto-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229588AbiFVIOy (ORCPT + 99 others); Wed, 22 Jun 2022 04:14:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41296 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229836AbiFVIOx (ORCPT ); Wed, 22 Jun 2022 04:14:53 -0400 Received: from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com [IPv6:2607:f8b0:4864:20::1036]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A9CF37BE8 for ; Wed, 22 Jun 2022 01:14:52 -0700 (PDT) Received: by mail-pj1-x1036.google.com with SMTP id p3-20020a17090a428300b001ec865eb4a2so10945206pjg.3 for ; Wed, 22 Jun 2022 01:14:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=kZTnS3VTllrLoaJqF/w8mR2VHKq3Lk7ph/8GKtgAs1s=; b=kwtFYGvf5kooG2LYNIEXgMNdUgRE2PTCmzJmp4fCELFGTNJ3ZyNNa8TjWnqpWfaXVD 7G9kCTpUy/GYrDCymhpu1QZDUkcd7WmQbLPGaXPr4inPEshWm0TwY/F9Plr8F1Wi+jt2 eRTtLt6grfJNlBaWbpMas44jwiDdBxXFI+G28HDXly5Tr8il7cx2B4Qe6rwShYKN/TSr hMT2nfAAICylX/8y3nSv7y9FINJEM78JgXndoTziueNznyT6SCLfbsqPqGTYOXiEZ1xT oOrMwZQpq2c14O60h2LwA7+WnU1zEYQtRHzCeBymb+wsiW0fMOCnrMnslpt4rSjVMF8d TDmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=kZTnS3VTllrLoaJqF/w8mR2VHKq3Lk7ph/8GKtgAs1s=; b=RRZfSASZEZi2rY3CajuPeQAWGLh9hZoPZUAWo0bGuthvdiv1MxJlE9LLTqWBQdUORo BraAYa8crmBnoKkpxZ0viyA+DoIcpnPVUj+awUjqF5nGgR0Rm8a8jS+tWi/WBNOY0Tl+ QQi4nhOSthXxuublIz0QCTjqBe+Vd0oGocrlJrgMp/uZ1fE2G+YpliAvqEDUN749t+ge cbGOsTKBKb0ED/8sjvwLNVybZhguH64rCGkJcWTRnPZFUVP9W/HI9OlmEqi2mBP2239P BJ/mGtMT46xfM0zGX/+9iW/KyOUL4vX0N8PbYxWVowk4/fobIkRup0VQa3wKc2BSTIuG rWfw== X-Gm-Message-State: AJIora+aSGeQ4T9VvT1ik6TJDRVoIgB1DPI6N/zUKiw6XrZ8Arsbd6NO Kq376pK5WO61bF5glyGzhxcsqA== X-Received: by 2002:a17:903:11c6:b0:167:90e5:59c2 with SMTP id q6-20020a17090311c600b0016790e559c2mr32466748plh.50.1655885691654; Wed, 22 Jun 2022 01:14:51 -0700 (PDT) Received: from [10.166.0.6] ([199.101.192.137]) by smtp.gmail.com with ESMTPSA id cp1-20020a170902e78100b00168f329b282sm12053860plb.155.2022.06.22.01.14.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Jun 2022 01:14:51 -0700 (PDT) Subject: Re: [PATCH] uacce: fix concurrency of fops_open and uacce_remove To: Greg Kroah-Hartman Cc: Jean-Philippe Brucker , Arnd Bergmann , Herbert Xu , Wangzhou , Jonathan Cameron , linux-accelerators@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org, iommu@lists.linux-foundation.org, Yang Shen References: <20220610123423.27496-1-zhangfei.gao@linaro.org> From: Zhangfei Gao Message-ID: <6fb8bed5-8d40-fd63-4537-44e9eb6aa053@linaro.org> Date: Wed, 22 Jun 2022 16:14:45 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org Hi, Greg On 2022/6/21 下午3:44, Greg Kroah-Hartman wrote: > On Tue, Jun 21, 2022 at 03:37:31PM +0800, Zhangfei Gao wrote: >> >> On 2022/6/20 下午9:36, Greg Kroah-Hartman wrote: >>> On Mon, Jun 20, 2022 at 02:24:31PM +0100, Jean-Philippe Brucker wrote: >>>> On Fri, Jun 17, 2022 at 02:05:21PM +0800, Zhangfei Gao wrote: >>>>>> The refcount only ensures that the uacce_device object is not freed as >>>>>> long as there are open fds. But uacce_remove() can run while there are >>>>>> open fds, or fds in the process of being opened. And atfer uacce_remove() >>>>>> runs, the uacce_device object still exists but is mostly unusable. For >>>>>> example once the module is freed, uacce->ops is not valid anymore. But >>>>>> currently uacce_fops_open() may dereference the ops in this case: >>>>>> >>>>>> uacce_fops_open() >>>>>> if (!uacce->parent->driver) >>>>>> /* Still valid, keep going */ >>>>>> ... rmmod >>>>>> uacce_remove() >>>>>> ... free_module() >>>>>> uacce->ops->get_queue() /* BUG */ >>>>> uacce_remove should wait for uacce->queues_lock, until fops_open release the >>>>> lock. >>>>> If open happen just after the uacce_remove: unlock, uacce_bind_queue in open >>>>> should fail. >>>> Ah yes sorry, I lost sight of what this patch was adding. But we could >>>> have the same issue with the patch, just in a different order, no? >>>> >>>> uacce_fops_open() >>>> uacce = xa_load() >>>> ... rmmod >>> Um, how is rmmod called if the file descriptor is open? >>> >>> That should not be possible if the owner of the file descriptor is >>> properly set. Please fix that up. >> Thanks Greg >> >> Set cdev owner or use module_get/put can block rmmod once fops_open. >> -       uacce->cdev->owner = THIS_MODULE; >> +       uacce->cdev->owner = uacce->parent->driver->owner; >> >> However, still not find good method to block removing parent pci device. >> >> $ echo 1 > /sys/bus/pci/devices/0000:00:02.0/remove & >> >> [   32.563350]  uacce_remove+0x6c/0x148 >> [   32.563353]  hisi_qm_uninit+0x12c/0x178 >> [   32.563356]  hisi_zip_remove+0xa0/0xd0 [hisi_zip] >> [   32.563361]  pci_device_remove+0x44/0xd8 >> [   32.563364]  device_remove+0x54/0x88 >> [   32.563367]  device_release_driver_internal+0xec/0x1a0 >> [   32.563370]  device_release_driver+0x20/0x30 >> [   32.563372]  pci_stop_bus_device+0x8c/0xe0 >> [   32.563375]  pci_stop_and_remove_bus_device_locked+0x28/0x60 >> [   32.563378]  remove_store+0x9c/0xb0 >> [   32.563379]  dev_attr_store+0x20/0x38 > Removing the parent pci device does not remove the module code, it > removes the device itself. Don't confuse code vs. data here. Do you mean even parent pci device is removed immediately, the code has to wait, like dma etc? Currently parent driver has to ensure all dma stopped then call uacce_remove, ie, after uacce_fops_open succeed, parent driver need wait fops_release, then uacce_remove can be called. For example: drivers/crypto/hisilicon/zip/zip_main.c: hisi_qm_wait_task_finish If remove this wait , there may other issue, Unable to handle kernel paging request at virtual address ffff80000b700204 pc : hisi_qm_cache_wb.part.0+0x2c/0xa0 So uacce only need serialize uacce_fops_open and uacce_remove. After uacce_fops_open, we can assume uacce_remove only happen after uacce_fops_release? Then it would be much simpler. Thanks