Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Wed, 25 Oct 2023 15:04:59 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Li Zhijian <lizhijian@fujitsu.com>, <alison.schofield@intel.com>,
        <vishal.l.verma@intel.com>, <ira.weiny@intel.com>,
        <bwidawsk@kernel.org>, <dan.j.williams@intel.com>,
        <linux-cxl@vger.kernel.org>
CC:     <dave.jiang@intel.com>, <Jonathan.Cameron@huawei.com>,
        <fan.ni@samsung.com>, <linux-kernel@vger.kernel.org>,
        Li Zhijian <lizhijian@fujitsu.com>
Subject: RE: [PATCH] cxl/region: Fix cxl_region_rwsem lock held when
 returning to user space
Message-ID: <6539910b1ae5c_780ef2949e@dwillia2-xfh.jf.intel.com.notmuch>
References: <20231025085450.2514906-1-lizhijian@fujitsu.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231025085450.2514906-1-lizhijian@fujitsu.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?BPnoJri7FYW5g5DdbUbTrJLOL+mGbf2oQMbzs8piv+bqYgcUi/qHRv0d3W4N?=
 =?us-ascii?Q?hZVLpnWkzdMJf8ZiEXkWACM1R6N3ZQ73fL1ht9fZyj466zJgiGW6BXz0H1UP?=
 =?us-ascii?Q?reE8cO0VdKnxOj2htCJ8Eu72x/tl9ctwnmhZ3WsGdlYzkqbhqnaVvp2m1TCv?=
 =?us-ascii?Q?zOlc/ydfLCeTEaA4wCpwMJMrI7kFIzUf8nA5H61//4S9ekdkW/i+3isbIxiQ?=
 =?us-ascii?Q?yeBTbk3PaPOOby8vwd99+ytQJlKDbHdq+0kHERCd00FHROc1UAueA/jBCvHB?=
 =?us-ascii?Q?nC+RuKDf8IZ1XJrxB2eOx19AUs79wC+4PP4F+OTbIbHAuAIfCwqC6A6sIVRX?=
 =?us-ascii?Q?Dh6SrMq/FyXtxSozQ5ej6VcChOIeytqOfLsKOosNd8D8EsTpPhX/PlBAZTYQ?=
 =?us-ascii?Q?OWZqMRBkE8pKvVpKGcLVE/vSQCvmxmdkumhBD27OKhOCTk9u59hdZfRc/Xnq?=
 =?us-ascii?Q?N3dsJwjcH6s9gCCMwDBxHl0oIuwNHEWHOTqKwqM5Cey5vetdBuJRpszbbThJ?=
 =?us-ascii?Q?c1cFOXrclK23GmQneMTtLuYbAU6EuWBnrZnlJpYeBv4/jOu0avMYw48K4yHD?=
 =?us-ascii?Q?xOb+3tcjBk0SsECWCFuaIYAnYZcwxsuzHe10OYOOd2O6by0/G1vb5xZIlmj5?=
 =?us-ascii?Q?FMbz1Y3pttjF6iFNS/FwG33QqA8e9O+d8poqFphYpKxsztFPVSoDEXHM1Yhf?=
 =?us-ascii?Q?uS+bTfpyIwuzrFCvVJxpWReW+Mee09mBqU8AzuaGCLKxALvTihH6uI30enHT?=
 =?us-ascii?Q?0jTrxsowSMX36NrTTk9Lnrv+Ay0gJ4jaf6DXScU0MloYelArVH9EsxK3HtVA?=
 =?us-ascii?Q?RjmJYNkD4SnJeafOF1Jc2Ll9B/UyeGcWOnd8LdWpbrE8Udn8sQp/gJYC335R?=
 =?us-ascii?Q?+wostzfbL+WLQZiXBdFYsd9Xh2BVMF1nSNi9q+u+ByiYNRpiXHA+CB+cOCpE?=
 =?us-ascii?Q?OcSWRq22HbtR5/eRaY7P0GQ2dKERfcYfeXu5zd5TsGKwKLT3rNO5WpRfEp7e?=
 =?us-ascii?Q?Zn4ygB49COH/lMVXwgUe2cwMOmZDkAXSOrs9a6KodytMjAs6kGcxTTn7l7vc?=
 =?us-ascii?Q?sM0zmTk0xt36UXTiQq2oaUAILPsZR0COoGhawnKCIPyLAFH7NBQ+d4HHJViX?=
 =?us-ascii?Q?qxIeyh/fbb/X1DsD0gtJOleEFwKR9/e5Oe24XDNuFlChApBaYrWCRa3uWqIk?=
 =?us-ascii?Q?imy6mEWpDphK1fhS9BSIJcd/KFBRJwWXq0qlMzJYnkdkAmeRgLT2aT/WvZP/?=
 =?us-ascii?Q?rbrpLitVVF2TeOAfZOgR3wlXlGA8Jjn+vPCe+zDZqqKSbMiyEFiCvqSMlgNj?=
 =?us-ascii?Q?NTLVxvXQhjjSqvY043IPK07eLxD8MAH5KNDf5zKX4VuN1FIqLkfhhmGpkPC2?=
 =?us-ascii?Q?GZGfPyNBYmZmgt3NrNVM0VPvL3viOmzcetE6k8kRIJDT0MLYcpX/aMw1T2eF?=
 =?us-ascii?Q?VjnHZ3drwAByjQusBg1NtZPmFc6H42S7Gxk4wduGG4NKfnhaNduUGgCSwWGX?=
 =?us-ascii?Q?0x+aORI8bs6CcJAGqrecZHXXAcZ4VpwDmTecqb6mPds3NfIVR13vRhQqQpgd?=
 =?us-ascii?Q?vJinv/H0dXUrzunqhGfeesSTYwXL32q8cecq4GlsbS1fY+HxBNR532nuR6Hp?=
 =?us-ascii?Q?kQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 449f5b8a-135d-4260-8f12-08dbd5a66f81
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Oct 2023 22:05:01.6958
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: rbc3Tctw5opk3OS+nMWwtRNgcZ28vz5LLoFvvZHFGakiomtt7WmjewOGfGB0NdUC/MnnL44CPi88gyWrlyv7lz+y5A+/FXFw1qgopHO1dQ4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB7053
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE autolearn=ham
        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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 25 Oct 2023 15:05:16 -0700 (PDT)

Li Zhijian wrote:
> goto out to release cxl_region_rwsem. Fix below warning:

Hi, this change looks good, the changelog would benefit from more
detail so that backporters can determine how important this fix is. I
will add the following:

---

cxl_region_invalidate_memregion() on x86 invokes wbinvd which is a
problematic instruction for virtualized environments. So, on virtualized
x86, cxl_region_invalidate_memregion() returns an error. This failure
case got missed because CXL memory-expander device passthrough is not a
production use case, and emulation of CXL devices is typically limited
to kernel development builds with CONFIG_CXL_REGION_INVALIDATION_TEST=y,
that makes cxl_region_invalidate_memregion() succeed.

In other words, the expected exposure of this bug is limited to CXL
subsystem development environments using QEMU that neglected
CONFIG_CXL_REGION_INVALIDATION_TEST=y.

> ================================================
> WARNING: lock held when returning to user space!
> 6.6.0-rc3-lizhijian+ #213 Not tainted
> ------------------------------------------------
> cxl/673 is leaving the kernel with locks still held!
> 1 lock held by cxl/673:
>  #0: ffffffffa013b9d0 (cxl_region_rwsem){++++}-{3:3}, at: commit_store+0x7d/0x3e0 [cxl_core]
> 
> Fixes: d1257d098a5a ("cxl/region: Move cache invalidation before region teardown, and before setup")
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
>  drivers/cxl/core/region.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6d63b8798c29..18da77f8f22b 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -294,7 +294,7 @@ static ssize_t commit_store(struct device *dev, struct device_attribute *attr,
>  	 */
>  	rc = cxl_region_invalidate_memregion(cxlr);
>  	if (rc)
> -		return rc;
> +		goto out;
>  
>  	if (commit) {
>  		rc = cxl_region_decode_commit(cxlr);
> -- 
> 2.29.2
>