pastebin - collaborative debugging tool
rovema.kpaste.net RSS


msnfs41client: Fix kernel ACL cache+misc, 2025-06-26
Posted by Anonymous on Thu 26th Jun 2025 18:04
raw | new post

  1. From fe15550b67e1d51c326f51d06c33beb247f0d9b3 Mon Sep 17 00:00:00 2001
  2. From: Roland Mainz <roland.mainz@nrubsig.org>
  3. Date: Thu, 26 Jun 2025 18:39:52 +0200
  4. Subject: [PATCH 1/2] sys: Fix kernel ACL cache
  5.  
  6. Fix kernel ACL cache.
  7.  
  8. This cache has only the lifetime of an open file handle, it's intended
  9. to avoid server roundtrips in case a program queries the ACL many times
  10. for the same file handle.
  11.  
  12. Reported-by: Dan Shelton <dan.f.shelton@gmail.com>
  13. Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
  14. ---
  15. sys/nfs41sys_acl.c | 88 ++++++++++++++++++++++++++++++++++------------
  16.  1 file changed, 65 insertions(+), 23 deletions(-)
  17.  
  18. diff --git a/sys/nfs41sys_acl.c b/sys/nfs41sys_acl.c
  19. index c468433..fce29f5 100644
  20. --- a/sys/nfs41sys_acl.c
  21. +++ b/sys/nfs41sys_acl.c
  22. @@ -213,6 +213,8 @@ NTSTATUS nfs41_QuerySecurityInformation(
  23.          NFS41GetNetRootExtension(SrvOpen->pVNetRoot->pNetRoot);
  24.      SECURITY_INFORMATION info_class =
  25.          RxContext->CurrentIrpSp->Parameters.QuerySecurity.SecurityInformation;
  26. +    ULONG querysecuritylength =
  27. +        RxContext->CurrentIrpSp->Parameters.QuerySecurity.Length;
  28.  #ifdef ENABLE_TIMINGS
  29.      LARGE_INTEGER t1, t2;
  30.      t1 = KeQueryPerformanceCounter(NULL);
  31. @@ -235,7 +237,19 @@ NTSTATUS nfs41_QuerySecurityInformation(
  32.              (long long)current_time.QuadPart,
  33.              (long long)nfs41_fobx->time.QuadPart);
  34.  #endif
  35. -        if (current_time.QuadPart - nfs41_fobx->time.QuadPart <= 20*1000) {
  36. +        if ((current_time.QuadPart - nfs41_fobx->time.QuadPart) <= (10*1000)) {
  37. +            if (querysecuritylength < nfs41_fobx->acl_len) {
  38. +                status = STATUS_BUFFER_OVERFLOW;
  39. +                RxContext->InformationToReturn = nfs41_fobx->acl_len;
  40. +
  41. +                DbgP("nfs41_QuerySecurityInformation: "
  42. +                    "STATUS_BUFFER_OVERFLOW for cached entry, "
  43. +                    "got %lu, need %lu\n",
  44. +                    (unsigned long)querysecuritylength,
  45. +                    (unsigned long)nfs41_fobx->acl_len);
  46. +                goto out;
  47. +            }
  48. +
  49.              PSECURITY_DESCRIPTOR sec_desc = (PSECURITY_DESCRIPTOR)
  50.                  RxContext->CurrentIrp->UserBuffer;
  51.              RtlCopyMemory(sec_desc, nfs41_fobx->acl, nfs41_fobx->acl_len);
  52. @@ -246,12 +260,17 @@ NTSTATUS nfs41_QuerySecurityInformation(
  53.              InterlockedIncrement(&getacl.sops);
  54.              InterlockedAdd64(&getacl.size, nfs41_fobx->acl_len);
  55.  #endif
  56. -        } else status = 1;
  57. -        RxFreePool(nfs41_fobx->acl);
  58. -        nfs41_fobx->acl = NULL;
  59. -        nfs41_fobx->acl_len = 0;
  60. -        if (!status)
  61. +
  62. +            DbgP("nfs41_QuerySecurityInformation: using cached ACL info\n");
  63.              goto out;
  64. +        } else {
  65. +            if (nfs41_fobx->acl) {
  66. +                RxFreePool(nfs41_fobx->acl);
  67. +                nfs41_fobx->acl = NULL;
  68. +                nfs41_fobx->acl_len = 0;
  69. +            }
  70. +            DbgP("nfs41_QuerySecurityInformation: cached ACL info invalidated\n");
  71. +        }
  72.      }
  73.  
  74.      status = nfs41_UpcallCreate(NFS41_SYSOP_ACL_QUERY, &nfs41_fobx->sec_ctx,
  75. @@ -263,7 +282,7 @@ NTSTATUS nfs41_QuerySecurityInformation(
  76.      /* we can't provide RxContext->CurrentIrp->UserBuffer to the upcall thread
  77.       * because it becomes an invalid pointer with that execution context
  78.       */
  79. -    entry->buf_len = RxContext->CurrentIrpSp->Parameters.QuerySecurity.Length;
  80. +    entry->buf_len = querysecuritylength;
  81.  
  82.      status = nfs41_UpcallWaitForReply(entry, pVNetRootContext->timeout);
  83.      if (status) {
  84. @@ -273,36 +292,52 @@ NTSTATUS nfs41_QuerySecurityInformation(
  85.      }
  86.  
  87.      if (entry->status == STATUS_BUFFER_TOO_SMALL) {
  88. -#ifdef DEBUG_ACL_QUERY
  89. -        DbgP("nfs41_QuerySecurityInformation: provided buffer size=%d but we "
  90. -             "need %lu\n",
  91. -             RxContext->CurrentIrpSp->Parameters.QuerySecurity.Length,
  92. -             entry->buf_len);
  93. -#endif
  94. +        DbgP("nfs41_QuerySecurityInformation: "
  95. +            "STATUS_BUFFER_OVERFLOW for entry, "
  96. +            "got %lu, need %lu\n",
  97. +            (unsigned long)querysecuritylength,
  98. +            (unsigned long)entry->buf_len);
  99.          status = STATUS_BUFFER_OVERFLOW;
  100.          RxContext->InformationToReturn = entry->buf_len;
  101.  
  102. -        /* Save ACL buffer */
  103. +        if (entry->buf) {
  104. +            RxFreePool(entry->buf);
  105. +            entry->buf = NULL;
  106. +        }
  107. +    } else if (entry->status == STATUS_SUCCESS) {
  108. +        /*
  109. +         * Free previous ACL data. This can happen if two concurrent
  110. +         * requests are executed for the same file
  111. +         */
  112. +        if (nfs41_fobx->acl) {
  113. +            RxFreePool(nfs41_fobx->acl);
  114. +            nfs41_fobx->acl = NULL;
  115. +            nfs41_fobx->acl_len = 0;
  116. +        }
  117. +
  118.          nfs41_fobx->acl = entry->buf;
  119.          nfs41_fobx->acl_len = entry->buf_len;
  120. +        entry->buf = NULL;
  121.          KeQuerySystemTime(&nfs41_fobx->time);
  122. -    } else if (entry->status == STATUS_SUCCESS) {
  123. +
  124.          PSECURITY_DESCRIPTOR sec_desc = (PSECURITY_DESCRIPTOR)
  125.              RxContext->CurrentIrp->UserBuffer;
  126. -        RtlCopyMemory(sec_desc, entry->buf, entry->buf_len);
  127. +        RtlCopyMemory(sec_desc, nfs41_fobx->acl, nfs41_fobx->acl_len);
  128. +        RxContext->IoStatusBlock.Information =
  129. +            RxContext->InformationToReturn = nfs41_fobx->acl_len;
  130. +        RxContext->IoStatusBlock.Status = status = STATUS_SUCCESS;
  131. +
  132.  #ifdef ENABLE_TIMINGS
  133.          InterlockedIncrement(&getacl.sops);
  134.          InterlockedAdd64(&getacl.size, entry->u.Acl.buf_len);
  135.  #endif
  136. -        RxFreePool(entry->buf);
  137. -        entry->buf = NULL;
  138. -        nfs41_fobx->acl = NULL;
  139. -        nfs41_fobx->acl_len = 0;
  140. -        RxContext->IoStatusBlock.Information = RxContext->InformationToReturn =
  141. -            entry->buf_len;
  142. -        RxContext->IoStatusBlock.Status = status = STATUS_SUCCESS;
  143.      } else {
  144.          status = map_query_acl_error(entry->status);
  145. +
  146. +        if (entry->buf) {
  147. +            RxFreePool(entry->buf);
  148. +            entry->buf = NULL;
  149. +        }
  150.      }
  151.  out:
  152.      if (entry) {
  153. @@ -411,6 +446,13 @@ NTSTATUS nfs41_SetSecurityInformation(
  154.      InterlockedAdd64(&setacl.size, entry->u.Acl.buf_len);
  155.  #endif
  156.  
  157. +    /* Invalidate cached ACL info */
  158. +    if (nfs41_fobx->acl) {
  159. +        RxFreePool(nfs41_fobx->acl);
  160. +        nfs41_fobx->acl = NULL;
  161. +        nfs41_fobx->acl_len = 0;
  162. +    }
  163. +
  164.      status = nfs41_UpcallWaitForReply(entry, pVNetRootContext->timeout);
  165.      if (status) {
  166.          /* Timeout - |nfs41_downcall()| will free |entry|+contents */
  167. --
  168. 2.45.1
  169.  
  170. From 730dd47583bcb52a73c980b13a529d6291c374b5 Mon Sep 17 00:00:00 2001
  171. From: Roland Mainz <roland.mainz@nrubsig.org>
  172. Date: Thu, 26 Jun 2025 18:42:14 +0200
  173. Subject: [PATCH 2/2] daemon: symlink code should have a #define for debug
  174.  level
  175.  
  176. symlink code should have a #define for debug level instead of hardcoding
  177. the values.
  178.  
  179. Signed-off-by: Cedric Blancher <cedric.blancher@gmail.com>
  180. ---
  181. daemon/symlink.c | 25 ++++++++++++++-----------
  182.  1 file changed, 14 insertions(+), 11 deletions(-)
  183.  
  184. diff --git a/daemon/symlink.c b/daemon/symlink.c
  185. index 29eea7f..78ab40c 100644
  186. --- a/daemon/symlink.c
  187. +++ b/daemon/symlink.c
  188. @@ -30,6 +30,9 @@
  189.  #include "util.h"
  190.  #include "daemon_debug.h"
  191.  
  192. +/* |DPRINTF()| levels for acl logging */
  193. +#define SYMLLVL1 1
  194. +#define SYMLLVL2 2
  195.  
  196.  static int abs_path_link(
  197.      OUT nfs41_abs_path *path,
  198. @@ -43,7 +46,7 @@ static int abs_path_link(
  199.      const char *link_end = link + link_len;
  200.      int status = NO_ERROR;
  201.  
  202. -    DPRINTF(2,
  203. +    DPRINTF(SYMLLVL2,
  204.          ("--> abs_path_link(path_pos='%s', link='%.*s', link_len=%d)\n",
  205.          path_pos, (int)link_len, link, (int)link_len));
  206.  
  207. @@ -103,12 +106,12 @@ out:
  208.      path->len = (unsigned short)(path_pos - path->path);
  209.  
  210.      if (status) {
  211. -        DPRINTF(2,
  212. +        DPRINTF(SYMLLVL2,
  213.              ("<-- abs_path_link(), status=%d\n",
  214.              status));
  215.      }
  216.      else {
  217. -        DPRINTF(2,
  218. +        DPRINTF(SYMLLVL2,
  219.              ("<-- abs_path_link(path='%.*s'), status=%d\n",
  220.              (int)path->len, path->path, status));
  221.      }
  222. @@ -136,7 +139,7 @@ int nfs41_symlink_target(
  223.          goto out;
  224.      }
  225.  
  226. -    DPRINTF(2, ("--> nfs41_symlink_target('%s', '%s')\n", path->path, link));
  227. +    DPRINTF(SYMLLVL2, ("--> nfs41_symlink_target('%s', '%s')\n", path->path, link));
  228.  
  229.      /* append any components after the symlink */
  230.      if (FAILED(StringCchCatA(link, NFS41_MAX_PATH_LEN,
  231. @@ -165,7 +168,7 @@ int nfs41_symlink_target(
  232.          goto out;
  233.      }
  234.  out:
  235. -    DPRINTF(2, ("<-- nfs41_symlink_target('%s') returning %d\n",
  236. +    DPRINTF(SYMLLVL2, ("<-- nfs41_symlink_target('%s') returning %d\n",
  237.          target->path, status));
  238.      return status;
  239.  }
  240. @@ -184,7 +187,7 @@ int nfs41_symlink_follow(
  241.      file.path = &path;
  242.      InitializeSRWLock(&path.lock);
  243.  
  244. -    DPRINTF(2, ("--> nfs41_symlink_follow('%s')\n", symlink->path->path));
  245. +    DPRINTF(SYMLLVL2, ("--> nfs41_symlink_follow('%s')\n", symlink->path->path));
  246.  
  247.      do {
  248.          if (++depth > NFS41_MAX_SYMLINK_DEPTH) {
  249. @@ -196,7 +199,7 @@ int nfs41_symlink_follow(
  250.          status = nfs41_symlink_target(session, symlink, &path);
  251.          if (status) goto out;
  252.  
  253. -        DPRINTF(2, ("looking up '%s'\n", path.path));
  254. +        DPRINTF(SYMLLVL2, ("looking up '%s'\n", path.path));
  255.  
  256.          last_component(path.path, path.path + path.len, &file.name);
  257.  
  258. @@ -208,7 +211,7 @@ int nfs41_symlink_follow(
  259.          symlink = &file;
  260.      } while (info->type == NF4LNK);
  261.  out:
  262. -    DPRINTF(2, ("<-- nfs41_symlink_follow() returning %d\n", status));
  263. +    DPRINTF(SYMLLVL2, ("<-- nfs41_symlink_follow() returning %d\n", status));
  264.      return status;
  265.  }
  266.  
  267. @@ -226,7 +229,7 @@ static int parse_symlink_get(unsigned char *buffer, uint32_t length,
  268.  
  269.      args->target_set = NULL;
  270.  
  271. -    DPRINTF(1,
  272. +    DPRINTF(SYMLLVL1,
  273.          ("parse_symlink_get: parsing NFS41_SYSOP_SYMLINK_GET: "
  274.          "path='%s' target='%s'\n",
  275.          args->path, args->target_set));
  276. @@ -254,7 +257,7 @@ static int handle_symlink_get(void *daemon_context, nfs41_upcall *upcall)
  277.          goto out;
  278.      }
  279.      args->target_get.len = (unsigned short)len;
  280. -    DPRINTF(2,
  281. +    DPRINTF(SYMLLVL2,
  282.          ("returning symlink target '%s'\n", args->target_get.path));
  283.  
  284.  out:
  285. @@ -316,7 +319,7 @@ static int parse_symlink_set(unsigned char *buffer, uint32_t length,
  286.      status = get_name(&buffer, &length,
  287.          (const char **)(&args->target_set));
  288.  
  289. -    DPRINTF(1,
  290. +    DPRINTF(SYMLLVL1,
  291.          ("parse_symlink_set: parsing NFS41_SYSOP_SYMLINK_SET: "
  292.          "path='%s' target='%s'\n",
  293.          args->path, args->target_set));
  294. --
  295. 2.45.1

Submit a correction or amendment below (click here to make a fresh posting)
After submitting an amendment, you'll be able to view the differences between the old and new posts easily.

Syntax highlighting:

To highlight particular lines, prefix each line with {%HIGHLIGHT}




All content is user-submitted.
The administrators of this site (kpaste.net) are not responsible for their content.
Abuse reports should be emailed to us at