Skip to content

Commit 7272093

Browse files
guilhermepiccolihansendc
authored andcommitted
x86/split_lock: Add sysctl to control the misery mode
Commit b041b52 ("x86/split_lock: Make life miserable for split lockers") changed the way the split lock detector works when in "warn" mode; basically, it not only shows the warn message, but also intentionally introduces a slowdown through sleeping plus serialization mechanism on such task. Based on discussions in [0], seems the warning alone wasn't enough motivation for userspace developers to fix their applications. This slowdown is enough to totally break some proprietary (aka. unfixable) userspace[1]. Happens that originally the proposal in [0] was to add a new mode which would warns + slowdown the "split locking" task, keeping the old warn mode untouched. In the end, that idea was discarded and the regular/default "warn" mode now slows down the applications. This is quite aggressive with regards proprietary/legacy programs that basically are unable to properly run in kernel with this change. While it is understandable that a malicious application could DoS by split locking, it seems unacceptable to regress old/proprietary userspace programs through a default configuration that previously worked. An example of such breakage was reported in [1]. Add a sysctl to allow controlling the "misery mode" behavior, as per Thomas suggestion on [2]. This way, users running legacy and/or proprietary software are allowed to still execute them with a decent performance while still observing the warning messages on kernel log. [0] https://lore.kernel.org/lkml/[email protected]/ [1] doitsujin/dxvk#2938 [2] https://lore.kernel.org/lkml/87pmf4bter.ffs@tglx/ [ dhansen: minor changelog tweaks, including clarifying the actual problem ] Fixes: b041b52 ("x86/split_lock: Make life miserable for split lockers") Suggested-by: Thomas Gleixner <[email protected]> Signed-off-by: Guilherme G. Piccoli <[email protected]> Signed-off-by: Dave Hansen <[email protected]> Reviewed-by: Tony Luck <[email protected]> Tested-by: Andre Almeida <[email protected]> Link: https://lore.kernel.org/all/20221024200254.635256-1-gpiccoli%40igalia.com
1 parent f0c4d9f commit 7272093

File tree

2 files changed

+76
-10
lines changed

2 files changed

+76
-10
lines changed

Documentation/admin-guide/sysctl/kernel.rst

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,6 +1314,29 @@ watchdog work to be queued by the watchdog timer function, otherwise the NMI
13141314
watchdog — if enabled — can detect a hard lockup condition.
13151315

13161316

1317+
split_lock_mitigate (x86 only)
1318+
==============================
1319+
1320+
On x86, each "split lock" imposes a system-wide performance penalty. On larger
1321+
systems, large numbers of split locks from unprivileged users can result in
1322+
denials of service to well-behaved and potentially more important users.
1323+
1324+
The kernel mitigates these bad users by detecting split locks and imposing
1325+
penalties: forcing them to wait and only allowing one core to execute split
1326+
locks at a time.
1327+
1328+
These mitigations can make those bad applications unbearably slow. Setting
1329+
split_lock_mitigate=0 may restore some application performance, but will also
1330+
increase system exposure to denial of service attacks from split lock users.
1331+
1332+
= ===================================================================
1333+
0 Disable the mitigation mode - just warns the split lock on kernel log
1334+
and exposes the system to denials of service from the split lockers.
1335+
1 Enable the mitigation mode (this is the default) - penalizes the split
1336+
lockers with intentional performance degradation.
1337+
= ===================================================================
1338+
1339+
13171340
stack_erasing
13181341
=============
13191342

arch/x86/kernel/cpu/intel.c

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,8 +1034,32 @@ static const struct {
10341034

10351035
static struct ratelimit_state bld_ratelimit;
10361036

1037+
static unsigned int sysctl_sld_mitigate = 1;
10371038
static DEFINE_SEMAPHORE(buslock_sem);
10381039

1040+
#ifdef CONFIG_PROC_SYSCTL
1041+
static struct ctl_table sld_sysctls[] = {
1042+
{
1043+
.procname = "split_lock_mitigate",
1044+
.data = &sysctl_sld_mitigate,
1045+
.maxlen = sizeof(unsigned int),
1046+
.mode = 0644,
1047+
.proc_handler = proc_douintvec_minmax,
1048+
.extra1 = SYSCTL_ZERO,
1049+
.extra2 = SYSCTL_ONE,
1050+
},
1051+
{}
1052+
};
1053+
1054+
static int __init sld_mitigate_sysctl_init(void)
1055+
{
1056+
register_sysctl_init("kernel", sld_sysctls);
1057+
return 0;
1058+
}
1059+
1060+
late_initcall(sld_mitigate_sysctl_init);
1061+
#endif
1062+
10391063
static inline bool match_option(const char *arg, int arglen, const char *opt)
10401064
{
10411065
int len = strlen(opt), ratelimit;
@@ -1146,12 +1170,20 @@ static void split_lock_init(void)
11461170
split_lock_verify_msr(sld_state != sld_off);
11471171
}
11481172

1149-
static void __split_lock_reenable(struct work_struct *work)
1173+
static void __split_lock_reenable_unlock(struct work_struct *work)
11501174
{
11511175
sld_update_msr(true);
11521176
up(&buslock_sem);
11531177
}
11541178

1179+
static DECLARE_DELAYED_WORK(sl_reenable_unlock, __split_lock_reenable_unlock);
1180+
1181+
static void __split_lock_reenable(struct work_struct *work)
1182+
{
1183+
sld_update_msr(true);
1184+
}
1185+
static DECLARE_DELAYED_WORK(sl_reenable, __split_lock_reenable);
1186+
11551187
/*
11561188
* If a CPU goes offline with pending delayed work to re-enable split lock
11571189
* detection then the delayed work will be executed on some other CPU. That
@@ -1169,25 +1201,36 @@ static int splitlock_cpu_offline(unsigned int cpu)
11691201
return 0;
11701202
}
11711203

1172-
static DECLARE_DELAYED_WORK(split_lock_reenable, __split_lock_reenable);
1173-
11741204
static void split_lock_warn(unsigned long ip)
11751205
{
1206+
struct delayed_work *work;
11761207
int cpu;
11771208

11781209
if (!current->reported_split_lock)
11791210
pr_warn_ratelimited("#AC: %s/%d took a split_lock trap at address: 0x%lx\n",
11801211
current->comm, current->pid, ip);
11811212
current->reported_split_lock = 1;
11821213

1183-
/* misery factor #1, sleep 10ms before trying to execute split lock */
1184-
if (msleep_interruptible(10) > 0)
1185-
return;
1186-
/* Misery factor #2, only allow one buslocked disabled core at a time */
1187-
if (down_interruptible(&buslock_sem) == -EINTR)
1188-
return;
1214+
if (sysctl_sld_mitigate) {
1215+
/*
1216+
* misery factor #1:
1217+
* sleep 10ms before trying to execute split lock.
1218+
*/
1219+
if (msleep_interruptible(10) > 0)
1220+
return;
1221+
/*
1222+
* Misery factor #2:
1223+
* only allow one buslocked disabled core at a time.
1224+
*/
1225+
if (down_interruptible(&buslock_sem) == -EINTR)
1226+
return;
1227+
work = &sl_reenable_unlock;
1228+
} else {
1229+
work = &sl_reenable;
1230+
}
1231+
11891232
cpu = get_cpu();
1190-
schedule_delayed_work_on(cpu, &split_lock_reenable, 2);
1233+
schedule_delayed_work_on(cpu, work, 2);
11911234

11921235
/* Disable split lock detection on this CPU to make progress */
11931236
sld_update_msr(false);

0 commit comments

Comments
 (0)