Re: [PATCH 4/6] cxlflash: Transition to application close model

2016-08-23 Thread Martin K. Petersen
> "Matthew" == Matthew R Ochs  writes:

Matthew> Caching the adapter file descriptor and performing a close on
Matthew> behalf of an application is a poor design. This is due to the
Matthew> fact that once a file descriptor in installed, it is free to be
Matthew> altered without the knowledge of the cxlflash driver. This can
Matthew> lead to inconsistencies between the application and
Matthew> kernel. Furthermore, the nature of the former design is more
Matthew> exploitable and thus should be abandoned.

Matthew> To support applications performing a close on the adapter file
Matthew> that is associated with a context, a new flag is introduced to
Matthew> the user API to indicate to applications that they are
Matthew> responsible for the close following the cleanup (detach) of a
Matthew> context. The documentation is also updated to reflect this
Matthew> change in behavior.

Applied patches 4 through 6 to 4.9/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/6] cxlflash: Transition to application close model

2016-08-18 Thread Manoj Kumar


Acked-by: Manoj N. Kumar 


On 8/9/2016 6:39 PM, Matthew R. Ochs wrote:

Caching the adapter file descriptor and performing a close on behalf
of an application is a poor design. This is due to the fact that once
a file descriptor in installed, it is free to be altered without the
knowledge of the cxlflash driver. This can lead to inconsistencies
between the application and kernel. Furthermore, the nature of the
former design is more exploitable and thus should be abandoned.

To support applications performing a close on the adapter file that
is associated with a context, a new flag is introduced to the user
API to indicate to applications that they are responsible for the
close following the cleanup (detach) of a context. The documentation
is also updated to reflect this change in behavior.

Inspired-by: Al Viro 
Signed-off-by: Matthew R. Ochs 


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/6] cxlflash: Transition to application close model

2016-08-09 Thread Matthew R. Ochs
Caching the adapter file descriptor and performing a close on behalf
of an application is a poor design. This is due to the fact that once
a file descriptor in installed, it is free to be altered without the
knowledge of the cxlflash driver. This can lead to inconsistencies
between the application and kernel. Furthermore, the nature of the
former design is more exploitable and thus should be abandoned.

To support applications performing a close on the adapter file that
is associated with a context, a new flag is introduced to the user
API to indicate to applications that they are responsible for the
close following the cleanup (detach) of a context. The documentation
is also updated to reflect this change in behavior.

Inspired-by: Al Viro 
Signed-off-by: Matthew R. Ochs 
---
 Documentation/powerpc/cxlflash.txt | 42 +++---
 drivers/scsi/cxlflash/superpipe.c  | 71 ++
 drivers/scsi/cxlflash/vlun.c   | 13 ++-
 include/uapi/scsi/cxlflash_ioctl.h | 19 +++---
 4 files changed, 73 insertions(+), 72 deletions(-)

diff --git a/Documentation/powerpc/cxlflash.txt 
b/Documentation/powerpc/cxlflash.txt
index 4202d1b..f4c1190 100644
--- a/Documentation/powerpc/cxlflash.txt
+++ b/Documentation/powerpc/cxlflash.txt
@@ -171,11 +171,30 @@ DK_CXLFLASH_ATTACH
   destroyed, the tokens are to be considered stale and subsequent
   usage will result in errors.
 
+   - A valid adapter file descriptor (fd2 >= 0) is only returned on
+ the initial attach for a context. Subsequent attaches to an
+ existing context (DK_CXLFLASH_ATTACH_REUSE_CONTEXT flag present)
+ do not provide the adapter file descriptor as it was previously
+ made known to the application.
+
 - When a context is no longer needed, the user shall detach from
-  the context via the DK_CXLFLASH_DETACH ioctl.
+  the context via the DK_CXLFLASH_DETACH ioctl. When this ioctl
+ returns with a valid adapter file descriptor and the return flag
+ DK_CXLFLASH_APP_CLOSE_ADAP_FD is present, the application _must_
+ close the adapter file descriptor following a successful detach.
+
+   - When this ioctl returns with a valid fd2 and the return flag
+ DK_CXLFLASH_APP_CLOSE_ADAP_FD is present, the application _must_
+ close fd2 in the following circumstances:
+
+   + Following a successful detach of the last user of the context
+   + Following a successful recovery on the context's original fd2
+   + In the child process of a fork(), following a clone ioctl,
+ on the fd2 associated with the source context
 
-- A close on fd2 will invalidate the tokens. This operation is not
-  required by the user.
+- At any time, a close on fd2 will invalidate the tokens. Applications
+ should exercise caution to only close fd2 when appropriate (outlined
+ in the previous bullet) to avoid premature loss of I/O.
 
 DK_CXLFLASH_USER_DIRECT
 ---
@@ -254,6 +273,10 @@ DK_CXLFLASH_DETACH
 success, all "tokens" which had been provided to the user from the
 DK_CXLFLASH_ATTACH onward are no longer valid.
 
+When the DK_CXLFLASH_APP_CLOSE_ADAP_FD flag was returned on a successful
+attach, the application _must_ close the fd2 associated with the context
+following the detach of the final user of the context.
+
 DK_CXLFLASH_VLUN_CLONE
 --
 This ioctl is responsible for cloning a previously created
@@ -261,7 +284,7 @@ DK_CXLFLASH_VLUN_CLONE
 support maintaining user space access to storage after a process
 forks. Upon success, the child process (which invoked the ioctl)
 will have access to the same LUNs via the same resource handle(s)
-and fd2 as the parent, but under a different context.
+as the parent, but under a different context.
 
 Context sharing across processes is not supported with CXL and
 therefore each fork must be met with establishing a new context
@@ -275,6 +298,12 @@ DK_CXLFLASH_VLUN_CLONE
 translation tables are copied from the parent context to the child's
 and then synced with the AFU.
 
+When the DK_CXLFLASH_APP_CLOSE_ADAP_FD flag was returned on a successful
+attach, the application _must_ close the fd2 associated with the source
+context (still resident/accessible in the parent process) following the
+clone. This is to avoid a stale entry in the file descriptor table of the
+child process.
+
 DK_CXLFLASH_VERIFY
 --
 This ioctl is used to detect various changes such as the capacity of
@@ -309,6 +338,11 @@ DK_CXLFLASH_RECOVER_AFU
 at which time the context/resources they held will be freed as part of
 the release fop.
 
+When the DK_CXLFLASH_APP_CLOSE_ADAP_FD flag was returned on a successful
+attach, the application _must_ unmap and close the fd2 associated with th