diff --git a/patches/server/0027-Synchronize-PaperPermissionManager.patch b/patches/server/0027-Synchronize-PaperPermissionManager.patch new file mode 100644 index 0000000..e849a94 --- /dev/null +++ b/patches/server/0027-Synchronize-PaperPermissionManager.patch @@ -0,0 +1,190 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Spottedleaf +Date: Mon, 15 May 2023 10:58:06 -0700 +Subject: [PATCH] Synchronize PaperPermissionManager + +Since multiple regions can exist, there are concurrent accesses +in this class. To prevent deadlock, the monitor is not held +when recalculating permissions, as Permissable holds its own +lock. + +This fixes CMEs originating from this class. + +diff --git a/src/main/java/io/papermc/paper/plugin/manager/PaperPermissionManager.java b/src/main/java/io/papermc/paper/plugin/manager/PaperPermissionManager.java +index 92a69677f21b2c1c035119d8e5a6af63fa19b801..6a239a3da5676cd781057d1b4af68aa97de27881 100644 +--- a/src/main/java/io/papermc/paper/plugin/manager/PaperPermissionManager.java ++++ b/src/main/java/io/papermc/paper/plugin/manager/PaperPermissionManager.java +@@ -32,7 +32,9 @@ abstract class PaperPermissionManager implements PermissionManager { + @Override + @Nullable + public Permission getPermission(@NotNull String name) { ++ synchronized (this) { // Folia - synchronized + return this.permissions().get(name.toLowerCase(java.util.Locale.ENGLISH)); ++ } // Folia - synchronized + } + + @Override +@@ -52,12 +54,24 @@ abstract class PaperPermissionManager implements PermissionManager { + private void addPermission(@NotNull Permission perm, boolean dirty) { + String name = perm.getName().toLowerCase(java.util.Locale.ENGLISH); + ++ Boolean recalc; // Folia - synchronized ++ synchronized (this) { // Folia - synchronized + if (this.permissions().containsKey(name)) { + throw new IllegalArgumentException("The permission " + name + " is already defined!"); + } + + this.permissions().put(name, perm); +- this.calculatePermissionDefault(perm, dirty); ++ recalc = this.calculatePermissionDefault(perm, dirty); ++ } // Folia - synchronized ++ // Folia start - synchronize this class - we hold a lock now, prevent deadlock by moving this out ++ if (recalc != null) { ++ if (recalc.booleanValue()) { ++ this.dirtyPermissibles(true); ++ } else { ++ this.dirtyPermissibles(false); ++ } ++ } ++ // Folia end - synchronize this class - we hold a lock now, prevent deadlock by moving this out + } + + @Override +@@ -80,42 +94,58 @@ abstract class PaperPermissionManager implements PermissionManager { + + @Override + public void recalculatePermissionDefaults(@NotNull Permission perm) { ++ Boolean recalc = null; // Folia - synchronized ++ synchronized (this) { // Folia - synchronized + // we need a null check here because some plugins for some unknown reason pass null into this? + if (perm != null && this.permissions().containsKey(perm.getName().toLowerCase(Locale.ENGLISH))) { + this.defaultPerms().get(true).remove(perm); + this.defaultPerms().get(false).remove(perm); + +- this.calculatePermissionDefault(perm, true); ++ recalc = this.calculatePermissionDefault(perm, true); // Folia - synchronized ++ } ++ } // Folia - synchronized ++ // Folia start - synchronize this class - we hold a lock now, prevent deadlock by moving this out ++ if (recalc != null) { ++ if (recalc.booleanValue()) { ++ this.dirtyPermissibles(true); ++ } else { ++ this.dirtyPermissibles(false); ++ } + } ++ // Folia end - synchronize this class - we hold a lock now, prevent deadlock by moving this out + } + +- private void calculatePermissionDefault(@NotNull Permission perm, boolean dirty) { ++ private Boolean calculatePermissionDefault(@NotNull Permission perm, boolean dirty) { // Folia - synchronize this class + if ((perm.getDefault() == PermissionDefault.OP) || (perm.getDefault() == PermissionDefault.TRUE)) { + this.defaultPerms().get(true).add(perm); + if (dirty) { +- this.dirtyPermissibles(true); ++ return Boolean.TRUE; // Folia - synchronize this class - we hold a lock now, prevent deadlock by moving this out + } + } + if ((perm.getDefault() == PermissionDefault.NOT_OP) || (perm.getDefault() == PermissionDefault.TRUE)) { + this.defaultPerms().get(false).add(perm); + if (dirty) { +- this.dirtyPermissibles(false); ++ return Boolean.FALSE; // Folia - synchronize this class - we hold a lock now, prevent deadlock by moving this out + } + } ++ return null; // Folia - synchronize this class + } + + + @Override + public void subscribeToPermission(@NotNull String permission, @NotNull Permissible permissible) { ++ synchronized (this) { // Folia - synchronized + String name = permission.toLowerCase(java.util.Locale.ENGLISH); + Map map = this.permSubs().computeIfAbsent(name, k -> new WeakHashMap<>()); + + map.put(permissible, true); ++ } // Folia - synchronized + } + + @Override + public void unsubscribeFromPermission(@NotNull String permission, @NotNull Permissible permissible) { + String name = permission.toLowerCase(java.util.Locale.ENGLISH); ++ synchronized (this) { // Folia - synchronized + Map map = this.permSubs().get(name); + + if (map != null) { +@@ -125,11 +155,13 @@ abstract class PaperPermissionManager implements PermissionManager { + this.permSubs().remove(name); + } + } ++ } // Folia - synchronized + } + + @Override + @NotNull + public Set getPermissionSubscriptions(@NotNull String permission) { ++ synchronized (this) { // Folia - synchronized + String name = permission.toLowerCase(java.util.Locale.ENGLISH); + Map map = this.permSubs().get(name); + +@@ -138,17 +170,21 @@ abstract class PaperPermissionManager implements PermissionManager { + } else { + return ImmutableSet.copyOf(map.keySet()); + } ++ } // Folia - synchronized + } + + @Override + public void subscribeToDefaultPerms(boolean op, @NotNull Permissible permissible) { ++ synchronized (this) { // Folia - synchronized + Map map = this.defSubs().computeIfAbsent(op, k -> new WeakHashMap<>()); + + map.put(permissible, true); ++ } // Folia - synchronized + } + + @Override + public void unsubscribeFromDefaultPerms(boolean op, @NotNull Permissible permissible) { ++ synchronized (this) { // Folia - synchronized + Map map = this.defSubs().get(op); + + if (map != null) { +@@ -158,11 +194,13 @@ abstract class PaperPermissionManager implements PermissionManager { + this.defSubs().remove(op); + } + } ++ } // Folia - synchronized + } + + @Override + @NotNull + public Set getDefaultPermSubscriptions(boolean op) { ++ synchronized (this) { // Folia - synchronized + Map map = this.defSubs().get(op); + + if (map == null) { +@@ -170,19 +208,24 @@ abstract class PaperPermissionManager implements PermissionManager { + } else { + return ImmutableSet.copyOf(map.keySet()); + } ++ } // Folia - synchronized + } + + @Override + @NotNull + public Set getPermissions() { ++ synchronized (this) { // Folia - synchronized + return new HashSet<>(this.permissions().values()); ++ } // Folia - synchronized + } + + @Override + public void clearPermissions() { ++ synchronized (this) { // Folia - synchronized + this.permissions().clear(); + this.defaultPerms().get(true).clear(); + this.defaultPerms().get(false).clear(); ++ } // Folia - synchronized + } + +