From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e23smtp03.au.ibm.com (e23smtp03.au.ibm.com [202.81.31.145]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id A78CE1A0E71 for ; Tue, 29 Sep 2015 10:54:54 +1000 (AEST) Received: from /spool/local by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 29 Sep 2015 10:54:52 +1000 Received: from d23relay07.au.ibm.com (d23relay07.au.ibm.com [9.190.26.37]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id F28DB2CE8054 for ; Tue, 29 Sep 2015 10:54:49 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay07.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t8T0sgN561210796 for ; Tue, 29 Sep 2015 10:54:51 +1000 Received: from d23av03.au.ibm.com (localhost [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t8T0sG8B007891 for ; Tue, 29 Sep 2015 10:54:17 +1000 Message-ID: <5609E137.6000301@au1.ibm.com> Date: Tue, 29 Sep 2015 10:54:15 +1000 From: Andrew Donnellan MIME-Version: 1.0 To: "Matthew R. Ochs" , linux-scsi@vger.kernel.org, James Bottomley , "Nicholas A. Bellinger" , Brian King , Ian Munsie , Daniel Axtens , Tomas Henzl , David Laight CC: Michael Neuling , "Manoj N. Kumar" , linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH v4 30/32] cxlflash: Fix to avoid corrupting adapter fops References: <1443222593-8828-1-git-send-email-mrochs@linux.vnet.ibm.com> <1443223164-10077-1-git-send-email-mrochs@linux.vnet.ibm.com> In-Reply-To: <1443223164-10077-1-git-send-email-mrochs@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 26/09/15 09:19, Matthew R. Ochs wrote: > The fops owned by the adapter can be corrupted in certain scenarios, > opening a window where certain fops are temporarily NULLed before being > reset to their proper value. This can potentially lead software to make > incorrect decisions, leaving the user with the inability to function as > intended. > > An example of this behavior can be observed when there are a number of > users with a high rate of turn around (attach to LUN, perform an I/O, > detach from LUN, repeat). Every so often a user is given a valid > context and adapter file descriptor, but the file associated with the > descriptor lacks the correct read permission bit (FMODE_CAN_READ) and > thus the read system call bails before calling the valid read fop. > > Background: > > The fops is stored in the adapter structure to provide the ability to > lookup the adapter structure from within the fop handler. CXL services > use the file's private_data and at present, the CXL context does not > have a private section. In an effort to limit areas of the cxlflash > driver with code specific the superpipe function, a design choice was > made to keep the details of the fops situated away from the legacy > portions of the driver. This drove the behavior that the adapter fops > is set at the beginning of the disk attach ioctl handler when there > are no users present. > > The corruption that this fix remedies is due to the fact that the fops > is initially defaulted to values found within a static structure. When > the fops is handed down to the CXL services later in the attach path, > certain services are patched. The fops structure remains correct until > the user count drops to 0 and the fops is reset, triggering the process > to repeat again. The user counts are tightly coupled with the creation > and deletion of the user context. If multiple users perform a disk > attach at the same time, when the user count is currently 0, some users > can be in the middle of obtaining a file descriptor and have not yet > reached the context creation code that [in addition to creating the > context] increments the user count. Subsequent users coming in to > perform the attach see that the user count is still 0, and reinitialize > the fops, temporarily removing the patched fops. The users that are in > the middle obtaining their file descriptor may then receive an invalid > descriptor. > > The fix simply removes the user count altogether and moves the fops > initialization to probe time such that it is only performed one time > for the life of the adapter. In the future, if the CXL services adopt > a private member for their context, that could be used to store the > adapter structure reference and cxlflash could revert to a model that > does not require an embedded fops. > > Signed-off-by: Matthew R. Ochs > Signed-off-by: Manoj N. Kumar Reviewed-by: Andrew Donnellan -- Andrew Donnellan Software Engineer, OzLabs andrew.donnellan@au1.ibm.com Australia Development Lab, Canberra +61 2 6201 8874 (work) IBM Australia Limited