ref counting for nfs41_open_state

daemon/getattr.c

@@ -71,6 +71,7 @@ static int parse_getattr(unsigned char *buffer, uint32_t length, nfs41_upcall *u
     if (status) goto out;
     status = safe_read(&buffer, &length, &args->state, sizeof(args->state));
     if (status) goto out;
+    upcall_open_state_ref(upcall, args->state);
 
     dprintf(1, "parsing NFS41_FILE_QUERY: info_class=%d buf_len=%d "
         "root=0x%p open_state=0x%p\n",

daemon/lock.c

@@ -94,6 +94,7 @@ static int parse_lock(unsigned char *buffer, uint32_t length, nfs41_upcall *upca
 
     status = safe_read(&buffer, &length, &args->state, sizeof(HANDLE));
     if (status) goto out;
+    upcall_open_state_ref(upcall, args->state);
     status = safe_read(&buffer, &length, &args->root, sizeof(HANDLE));
     if (status) goto out;
     status = safe_read(&buffer, &length, &args->offset, sizeof(LONGLONG));
@@ -181,6 +182,7 @@ static int parse_unlock(unsigned char *buffer, uint32_t length, nfs41_upcall *up
 
     status = safe_read(&buffer, &length, &args->state, sizeof(HANDLE));
     if (status) goto out;
+    upcall_open_state_ref(upcall, args->state);
     status = safe_read(&buffer, &length, &args->root, sizeof(HANDLE));
     if (status) goto out;
     status = safe_read(&buffer, &length, &args->count, sizeof(ULONG));

daemon/nfs41.h

@@ -72,6 +72,12 @@ typedef struct __nfs41_lock_state {
     SRWLOCK lock;
 } nfs41_lock_state;
 
+/* nfs41_open_state reference counting:
+ * one reference is held implicitly by the driver (initialized to 1 on
+ * OPEN and released on CLOSE). other references must be held during
+ * upcalls to prevent a parallel CLOSE from freeing it prematurely. by
+ * calling upcall_open_state_ref() when parsing the upcall, you are
+ * guaranteed a matching dereference on upcall_cleanup() */
 typedef struct __nfs41_open_state {
     nfs41_abs_path path;
     nfs41_path_fh parent;
@@ -85,6 +91,7 @@ typedef struct __nfs41_open_state {
     nfs41_lock_state last_lock;
     struct __pnfs_file_layout *layout;
     SRWLOCK lock;
+    LONG ref_count;
 } nfs41_open_state;
 
 typedef struct __nfs41_rpc_clnt {
@@ -355,4 +362,12 @@ static __inline netaddr4* nfs41_rpc_netaddr(
 
 bool_t nfs41_renew_in_progress(nfs41_client *client, bool_t *value);
 
+
+/* open.c */
+void nfs41_open_state_ref(
+    IN nfs41_open_state *state);
+
+void nfs41_open_state_deref(
+    IN nfs41_open_state *state);
+
 #endif /* __NFS41__ */

daemon/open.c

@@ -61,6 +61,7 @@ static int create_open_state(
         "%u", open_owner_id);
     state->owner.owner_len = (uint32_t)strlen(
         (const char*)state->owner.owner);
+    state->ref_count = 1;
 
     *state_out = state;
     status = NO_ERROR;
@@ -73,13 +74,33 @@ out_free:
 }
 
 static void free_open_state(
-    IN nfs41_session *session,
     IN nfs41_open_state *state)
 {
     free(state);
 }
 
 
+void nfs41_open_state_ref(
+    IN nfs41_open_state *state)
+{
+    const LONG count = InterlockedIncrement(&state->ref_count);
+
+    dprintf(2, "nfs41_open_state_ref(%s) count %d\n",
+        state->path.path, count);
+}
+
+void nfs41_open_state_deref(
+    IN nfs41_open_state *state)
+{
+    const LONG count = InterlockedDecrement(&state->ref_count);
+
+    dprintf(2, "nfs41_open_state_deref(%s) count %d\n",
+        state->path.path, count);
+    if (count == 0)
+        free_open_state(state);
+}
+
+
 /* NFS41_OPEN */
 static int parse_open(unsigned char *buffer, uint32_t length, nfs41_upcall *upcall)
 {
@@ -398,7 +419,7 @@ static int handle_open(nfs41_upcall *upcall)
 out:
     return status;
 out_free_state:
-    free(state);
+    nfs41_open_state_deref(state);
     goto out;
 }
 
@@ -461,7 +482,7 @@ static void cancel_open(IN nfs41_upcall *upcall)
                 nfs_error_string(status));
     }
 
-    free_open_state(state->session, state);
+    nfs41_open_state_deref(state);
 out:
     status = nfs_to_windows_error(status, ERROR_INTERNAL_ERROR);
     dprintf(1, "<-- cancel_open() returning %d\n", status);
@@ -531,13 +552,20 @@ static int handle_close(nfs41_upcall *upcall)
         }
     }
 
-    free_open_state(state->session, state);
     if (status || !rm_status)
         return status;
     else
         return rm_status;
 }
 
+static void cleanup_close(nfs41_upcall *upcall)
+{
+    close_upcall_args *args = &upcall->args.close;
+
+    /* release the initial reference from create_open_state() */
+    nfs41_open_state_deref(args->state);
+}
+
 
 const nfs41_upcall_op nfs41_op_open = {
     parse_open,
@@ -547,5 +575,8 @@ const nfs41_upcall_op nfs41_op_open = {
 };
 const nfs41_upcall_op nfs41_op_close = {
     parse_close,
-    handle_close
+    handle_close,
+    NULL,
+    NULL,
+    cleanup_close
 };

daemon/readdir.c

@@ -64,6 +64,7 @@ static int parse_readdir(unsigned char *buffer, uint32_t length, nfs41_upcall *u
     if (status) goto out;
     status = safe_read(&buffer, &length, &args->state, sizeof(args->state));
     if (status) goto out;
+    upcall_open_state_ref(upcall, args->state);
 
     dprintf(1, "parsing NFS41_DIR_QUERY: info_class=%d buf_len=%d "
         "filter='%s'\n\tInitial\\Restart\\Single %d\\%d\\%d "

daemon/readwrite.c

@@ -49,6 +49,7 @@ static int parse_rw(unsigned char *buffer, uint32_t length, nfs41_upcall *upcall
     if (status) goto out;
     status = safe_read(&buffer, &length, &args->state, sizeof(args->state));
     if (status) goto out;
+    upcall_open_state_ref(upcall, args->state);
 
     dprintf(1, "parsing %s len=%ld offset=%ld buf=%p root=%p "
         "open_state=0x%p\n", opcode2string(upcall->opcode), args->len,

daemon/setattr.c

@@ -56,6 +56,7 @@ static int parse_setattr(unsigned char *buffer, uint32_t length, nfs41_upcall *u
     if (status) goto out_free;
     status = safe_read(&buffer, &length, &args->state, sizeof(args->state));
     if (status) goto out_free;
+    upcall_open_state_ref(upcall, args->state);
     status = safe_read(&buffer, &length, &args->open_owner_id, sizeof(ULONG));
     if (status) goto out_free;
     status = safe_read(&buffer, &length, &args->access_mask, sizeof(ULONG));
@@ -501,6 +502,7 @@ static int parse_setexattr(unsigned char *buffer, uint32_t length, nfs41_upcall
     if (status) goto out;
     status = safe_read(&buffer, &length, &args->state, sizeof(args->state));
     if (status) goto out;
+    upcall_open_state_ref(upcall, args->state);
     status = safe_read(&buffer, &length, &args->mode, sizeof(args->mode));
     if (status) goto out;
 

daemon/symlink.c

@@ -199,6 +199,7 @@ static int parse_symlink(unsigned char *buffer, uint32_t length, nfs41_upcall *u
     if (status) goto out;
     status = safe_read(&buffer, &length, &args->state, sizeof(nfs41_open_state *));
     if (status) goto out;
+    upcall_open_state_ref(upcall, args->state);
     status = get_name(&buffer, &length, &args->path);
     if (status) goto out;
     status = safe_read(&buffer, &length, &args->set, sizeof(BOOLEAN));

daemon/upcall.c

@@ -181,4 +181,9 @@ void upcall_cleanup(
     const nfs41_upcall_op *op = g_upcall_op_table[upcall->opcode];
     if (op && op->cleanup)
         op->cleanup(upcall);
+
+    if (upcall->state_ref) {
+        nfs41_open_state_deref(upcall->state_ref);
+        upcall->state_ref = NULL;
+    }
 }

daemon/upcall.h

@@ -176,6 +176,11 @@ typedef struct __nfs41_upcall {
     uint32_t                status;
     uint32_t                last_error;
     upcall_args             args;
+
+    /* store referenced pointers with the upcall for
+     * automatic dereferencing on upcall_cleanup();
+     * see upcall_root_ref() and upcall_open_state_ref() */
+    nfs41_open_state        *state_ref;
 } nfs41_upcall;
 
 
@@ -216,4 +221,13 @@ void upcall_cancel(
 void upcall_cleanup(
     IN nfs41_upcall *upcall);
 
+
+static __inline void upcall_open_state_ref(
+    IN nfs41_upcall *upcall,
+    IN nfs41_open_state *state)
+{
+    nfs41_open_state_ref(state);
+    upcall->state_ref = state;
+}
+
 #endif /* !__NFS41_DAEMON_UPCALL_H__ */