system/nxpkg: make locks instance-safe

Assisted-by: OpenAI Codex:gpt-5.6-sol
Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>

Author: aviralgarg05

system/nxpkg/pkg.h

@@ -55,26 +55,15 @@
 #define PKG_DESCRIPTION_MAX   127
 #define PKG_CATEGORY_MAX      31
 #define PKG_HASH_HEX_LEN      64
-/* Each manifest slot is ~1.7KB (dominated by PKG_LAUNCH_ARGS_MAX slots).
- * Keep the catalog bounded so repository-provided metadata cannot cause
- * unbounded memory use.  Callers should allocate struct pkg_index_s from
- * the application heap rather than placing it on a small task stack.
- */
+/* Keep repository metadata memory bounded. */
 
 #define PKG_INDEX_MAX         16
 #define PKG_INSTALLED_MAX     16
 #define PKG_INSTALLED_VERSIONS_MAX 8
 #define PKG_LAUNCH_ARGS_MAX   8
 #define PKG_LAUNCH_ARG_MAX    127
 
-/* Caps against a malicious/compromised HTTP server: without these, an
- * oversized response can exhaust SD-card space (downloads) or force an
- * unbounded single heap allocation sized directly off attacker-controlled
- * content (pkg_store_read_text).  Text/metadata files (index.jsn,
- * instpkg.jsn) are always small; artifact downloads cover the largest
- * real payloads seen in practice (a multi-MB WAD, a ~1MB game ELF) with
- * generous headroom.
- */
+/* Bound metadata allocations and artifact downloads. */
 
 #define PKG_TEXT_MAX_SIZE     (256 * 1024)
 #define PKG_DOWNLOAD_MAX_SIZE (32 * 1024 * 1024)
@@ -243,9 +232,9 @@ int pkg_resolve_artifact_source(FAR char *buffer, size_t size,
                                 FAR const struct pkg_manifest_s *manifest);
 int pkg_resolve_icon_source(FAR char *buffer, size_t size,
                             FAR const struct pkg_manifest_s *manifest);
-int pkg_acquire_source(FAR const char *source, FAR const char *dest,
-                       FAR const char *renew_lock_path);
+int pkg_acquire_source(FAR const char *source, FAR const char *dest);
 int pkg_lock_create(FAR const char *path);
+void pkg_lock_remove(FAR const char *path);
 void pkg_reclaim_stale_lock(FAR const char *path);
 int pkg_sync(FAR const char *source);
 int pkg_install(FAR const char *name);

system/nxpkg/pkg_install.c

@@ -433,7 +433,7 @@ int pkg_install(FAR const char *name)
   pkg_txn_clear_state(name);
   if (lock[0] != '\0')
     {
-      pkg_store_remove_file(lock);
+      pkg_lock_remove(lock);
     }
 
   pkg_info("installed %s version %s", manifest->name, manifest->version);
@@ -451,7 +451,7 @@ int pkg_install(FAR const char *name)
   pkg_txn_clear_state(name);
   if (lock[0] != '\0')
     {
-      pkg_store_remove_file(lock);
+      pkg_lock_remove(lock);
     }
 
   free(index);

system/nxpkg/pkg_store.c

@@ -27,10 +27,7 @@
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
-#include <inttypes.h>
-#include <pthread.h>
 #include <signal.h>
-#include <stdint.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
@@ -44,51 +41,30 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
-#define PKG_LOCK_RECORD_MAGIC "NXPKG1"
-#define PKG_LOCK_RECORD_SIZE  64
-
-/****************************************************************************
- * Private Data
- ****************************************************************************/
-
-static pthread_once_t g_pkg_lock_boot_once = PTHREAD_ONCE_INIT;
-static uint64_t g_pkg_lock_boot_id;
+#define PKG_LOCK_OWNER "owner"
 
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
-static void pkg_lock_init_boot_id(void)
-{
-  arc4random_buf(&g_pkg_lock_boot_id, sizeof(g_pkg_lock_boot_id));
-  if (g_pkg_lock_boot_id == 0)
-    {
-      g_pkg_lock_boot_id = 1;
-    }
-}
-
-static uint64_t pkg_lock_get_boot_id(void)
-{
-  if (pthread_once(&g_pkg_lock_boot_once, pkg_lock_init_boot_id) != 0)
-    {
-      return 1;
-    }
-
-  return g_pkg_lock_boot_id;
-}
-
-static int pkg_lock_read_owner(FAR const char *path,
-                               FAR uint64_t *boot_id,
-                               FAR pid_t *owner)
+static int pkg_lock_read_owner(FAR const char *path, FAR pid_t *owner)
 {
-  char record[PKG_LOCK_RECORD_SIZE];
-  unsigned long long parsed_boot;
+  char owner_path[PATH_MAX];
+  char record[32];
+  char extra;
   long parsed_owner;
   ssize_t nread;
   int fd;
   int ret;
 
-  fd = open(path, O_RDONLY);
+  ret = snprintf(owner_path, sizeof(owner_path), "%s/%s", path,
+                 PKG_LOCK_OWNER);
+  if (ret < 0 || (size_t)ret >= sizeof(owner_path))
+    {
+      return ret < 0 ? ret : -ENAMETOOLONG;
+    }
+
+  fd = open(owner_path, O_RDONLY);
   if (fd < 0)
     {
       return -errno;
@@ -104,16 +80,13 @@ static int pkg_lock_read_owner(FAR const char *path,
 
   close(fd);
   record[nread] = '\0';
-
-  ret = sscanf(record, PKG_LOCK_RECORD_MAGIC " %llx %ld",
-               &parsed_boot, &parsed_owner);
-  if (ret != 2 || parsed_owner <= 0 ||
+  if (sscanf(record, "%ld%c", &parsed_owner, &extra) != 1 ||
+      parsed_owner <= 0 ||
       (long)(pid_t)parsed_owner != parsed_owner)
     {
       return -EINVAL;
     }
 
-  *boot_id = (uint64_t)parsed_boot;
   *owner = (pid_t)parsed_owner;
   return 0;
 }
@@ -274,78 +247,104 @@ int pkg_store_prepare_layout(void)
 
 int pkg_lock_create(FAR const char *path)
 {
-  char record[PKG_LOCK_RECORD_SIZE];
+  char owner_path[PATH_MAX];
+  char record[32];
   int fd;
   int ret;
 
-  fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0644);
-  if (fd < 0)
+  if (mkdir(path, 0755) < 0)
     {
       return -errno;
     }
 
-  ret = snprintf(record, sizeof(record), PKG_LOCK_RECORD_MAGIC
-                 " %016" PRIx64 " %ld\n",
-                 pkg_lock_get_boot_id(), (long)getpid());
-  if (ret < 0 || (size_t)ret >= sizeof(record))
+  ret = snprintf(owner_path, sizeof(owner_path), "%s/%s", path,
+                 PKG_LOCK_OWNER);
+  if (ret < 0 || (size_t)ret >= sizeof(owner_path))
     {
       ret = ret < 0 ? ret : -ENAMETOOLONG;
-      goto errout;
+      rmdir(path);
+      return ret;
     }
 
-  ret = pkg_store_write_all(fd, record, (size_t)ret);
-  if (ret < 0)
+  fd = open(owner_path, O_WRONLY | O_CREAT | O_EXCL, 0644);
+  if (fd < 0)
     {
-      goto errout;
+      ret = -errno;
+      rmdir(path);
+      return ret;
     }
 
-  if (fsync(fd) < 0)
+  ret = snprintf(record, sizeof(record), "%ld", (long)getpid());
+  if (ret < 0 || (size_t)ret >= sizeof(record))
+    {
+      ret = ret < 0 ? ret : -EOVERFLOW;
+    }
+  else
+    {
+      ret = pkg_store_write_all(fd, record, (size_t)ret);
+    }
+
+  if (ret == 0 && fsync(fd) < 0)
     {
       ret = -errno;
-      goto errout;
     }
 
-  if (close(fd) < 0)
+  if (close(fd) < 0 && ret == 0)
     {
       ret = -errno;
-      unlink(path);
-      return ret;
     }
 
-  return 0;
+  if (ret < 0)
+    {
+      unlink(owner_path);
+      rmdir(path);
+    }
 
-errout:
-  close(fd);
-  unlink(path);
   return ret;
 }
 
+void pkg_lock_remove(FAR const char *path)
+{
+  char owner_path[PATH_MAX];
+  int ret;
+
+  ret = snprintf(owner_path, sizeof(owner_path), "%s/%s", path,
+                 PKG_LOCK_OWNER);
+  if (ret >= 0 && (size_t)ret < sizeof(owner_path))
+    {
+      unlink(owner_path);
+    }
+
+  rmdir(path);
+}
+
 void pkg_reclaim_stale_lock(FAR const char *path)
 {
-  uint64_t boot_id;
   pid_t owner;
   int ret;
 
-  ret = pkg_lock_read_owner(path, &boot_id, &owner);
-  if (ret == -EINVAL)
+  ret = pkg_lock_read_owner(path, &owner);
+  if (ret == -ENOENT || ret == -EINVAL)
     {
-      /* The creator may have completed open(O_EXCL) without writing yet.
-       * Give that window time to close before reading again.
-       */
+      /* The creator may not have written the owner marker yet. */
 
       usleep(20 * 1000);
-      ret = pkg_lock_read_owner(path, &boot_id, &owner);
+      ret = pkg_lock_read_owner(path, &owner);
+      if (ret == -ENOENT || ret == -EINVAL)
+        {
+          pkg_lock_remove(path);
+          return;
+        }
     }
 
   if (ret < 0)
     {
       return;
     }
 
-  if (boot_id != pkg_lock_get_boot_id())
+  if (owner == getpid())
     {
-      pkg_error("reclaiming lock from an earlier boot '%s'", path);
-      unlink(path);
+      pkg_lock_remove(path);
       return;
     }
 
@@ -358,7 +357,7 @@ void pkg_reclaim_stale_lock(FAR const char *path)
     {
       pkg_error("reclaiming lock from exited task %ld '%s'",
                 (long)owner, path);
-      unlink(path);
+      pkg_lock_remove(path);
     }
 }
 
@@ -550,12 +549,7 @@ int pkg_store_read_text(FAR const char *path, FAR char **buffer)
       return -EINVAL;
     }
 
-  /* Reject anything unreasonably large before the size is trusted for an
-   * allocation: guards both against a malicious/oversized text file (this
-   * path is used for the network-fetched index.jsn) and against
-   * "length + 1" wrapping if st_size were ever attacker-influenced up to
-   * SIZE_MAX.
-   */
+  /* Validate the size before allocating length plus its terminator. */
 
   if (st.st_size < 0 || st.st_size > (off_t)PKG_TEXT_MAX_SIZE)
     {
@@ -616,10 +610,7 @@ int pkg_store_read_text(FAR const char *path, FAR char **buffer)
  * Name: pkg_store_make_tmp_path
  *
  * Description:
- *   Derive a staging path for an atomic write/copy to "path", under a
- *   short-name-compatible extension instead of appending ".tmp" (which
- *   would produce a second '.' in the final path component and break on
- *   FAT filesystems without long file name support).
+ *   Build a FAT short-name-compatible staging path.
  *
  ****************************************************************************/
 
@@ -711,14 +702,7 @@ int pkg_store_write_text_atomic(FAR const char *path, FAR const char *text)
       return ret;
     }
 
-  /* Force the write through to disk before renaming.  nxpkg writes
-   * several small, unrelated files back-to-back during install (per-
-   * package txn state, then the shared installed-packages database);
-   * without an explicit sync here, the FAT driver's single shared
-   * sector cache can still hold a not-yet-committed buffer for this
-   * file when the very next atomic write starts touching a different
-   * file, corrupting one or both.
-   */
+  /* Commit the temporary file before the atomic rename. */
 
   ret = fsync(fd);
   if (ret < 0)
@@ -815,11 +799,7 @@ int pkg_store_copy_file(FAR const char *src, FAR const char *dest)
   close(infd);
 
 #ifndef CONFIG_PSEUDOFS_FILE
-  /* Force the payload through to disk before renaming - this is the
-   * largest write in the whole install pipeline (WAD/game-ELF-sized
-   * payloads), so a hard power-loss here is the scenario the atomic
-   * temp+rename is specifically protecting against.
-   */
+  /* Commit the payload before the atomic rename. */
 
   if (fsync(outfd) < 0)
     {
@@ -875,15 +855,7 @@ int pkg_store_remove_version_dir(FAR const char *name,
   FAR struct dirent *ent;
   int ret;
 
-  /* Generic directory-content removal (rather than unlinking the payload
-   * and manifest.jsn by their known names) so this same helper works both
-   * to reclaim a partially staged version directory after a failed
-   * install (pkg_install.c) and to prune/remove a fully-installed
-   * version, without needing to already know that version's artifact
-   * filename.  Best-effort throughout: this runs from error/cleanup
-   * paths where a still-failing removal shouldn't itself abort the
-   * caller.
-   */
+  /* Remove every file in a staged or installed version directory. */
 
   ret = pkg_store_format_version_path(path, sizeof(path), name, version);
   if (ret < 0)