Upstream/Folia
1
0
Fork 0
mirror of https://github.com/PaperMC/Folia.git synced 2024-11-21 11:55:11 +01:00
Code Issues Packages Projects Releases Wiki Activity

Synchronize PaperPermissionManager

Browse Source 
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.
This commit is contained in:
Spottedleaf 2023-05-15 11:00:49 -07:00
parent 051ec0dd65
commit 80af54eeda
1 changed files with 190 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

190
patches/server/0027-Synchronize-PaperPermissionManager.patch Normal file
View File

@ -0,0 +1,190 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Spottedleaf <Spottedleaf@users.noreply.github.com>
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<Permissible, Boolean> 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<Permissible, Boolean> 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<Permissible> getPermissionSubscriptions(@NotNull String permission) {
+ synchronized (this) { // Folia - synchronized
String name = permission.toLowerCase(java.util.Locale.ENGLISH);
Map<Permissible, Boolean> 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<Permissible, Boolean> 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<Permissible, Boolean> 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<Permissible> getDefaultPermSubscriptions(boolean op) {
+ synchronized (this) { // Folia - synchronized
Map<Permissible, Boolean> 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<Permission> 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
}