| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1432903635-27272-1-git-send-email-jmarchan@redhat.com>
Date: Fri, 29 May 2015 14:47:15 +0200
From: "Jerome Marchand" <jmarchan@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>
Cc: linux-kernel@...r.kernel.org
Subject: [PATCH] fix a race between /proc/lock_stat and module unloading
When opening /proc/lock_stat, lock_stat_open() makes a copy of
all_lock_classes list in the form of an array of ad hoc structures
lock_stat_data that reference lock_class, so it can be sorted and
passed to seq_read(). However, nothing prevents module unloading code
to free some of these lock_class structures before seq_read() tries to
access them.
Copying the all lock_class structures instead of just their pointers
would be an easy fix, but it seems quite wasteful. This patch copies
only the needed data into the lock_stat_data structure.
Signed-off-by: Jerome Marchand <jmarchan@...hat.com>
---
kernel/locking/lockdep_proc.c | 88 ++++++++++++++++++++++++-------------------
1 file changed, 50 insertions(+), 38 deletions(-)
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index ef43ac4..c2eb8e8 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -363,7 +363,9 @@ static const struct file_operations proc_lockdep_stats_operations = {
#ifdef CONFIG_LOCK_STAT
struct lock_stat_data {
- struct lock_class *class;
+ char name[39];
+ unsigned long contention_point[LOCKSTAT_POINTS];
+ unsigned long contending_point[LOCKSTAT_POINTS];
struct lock_class_stats stats;
};
@@ -426,39 +428,12 @@ static void seq_lock_time(struct seq_file *m, struct lock_time *lt)
static void seq_stats(struct seq_file *m, struct lock_stat_data *data)
{
- char name[39];
- struct lock_class *class;
+ char *name = data->name;
struct lock_class_stats *stats;
- int i, namelen;
+ int i, namelen = strlen(data->name);
- class = data->class;
stats = &data->stats;
- namelen = 38;
- if (class->name_version > 1)
- namelen -= 2; /* XXX truncates versions > 9 */
- if (class->subclass)
- namelen -= 2;
-
- if (!class->name) {
- char str[KSYM_NAME_LEN];
- const char *key_name;
-
- key_name = __get_key_name(class->key, str);
- snprintf(name, namelen, "%s", key_name);
- } else {
- snprintf(name, namelen, "%s", class->name);
- }
- namelen = strlen(name);
- if (class->name_version > 1) {
- snprintf(name+namelen, 3, "#%d", class->name_version);
- namelen += 2;
- }
- if (class->subclass) {
- snprintf(name+namelen, 3, "/%d", class->subclass);
- namelen += 2;
- }
-
if (stats->write_holdtime.nr) {
if (stats->read_holdtime.nr)
seq_printf(m, "%38s-W:", name);
@@ -490,32 +465,32 @@ static void seq_stats(struct seq_file *m, struct lock_stat_data *data)
for (i = 0; i < LOCKSTAT_POINTS; i++) {
char ip[32];
- if (class->contention_point[i] == 0)
+ if (data->contention_point[i] == 0)
break;
if (!i)
seq_line(m, '-', 40-namelen, namelen);
snprintf(ip, sizeof(ip), "[<%p>]",
- (void *)class->contention_point[i]);
+ (void *)data->contention_point[i]);
seq_printf(m, "%40s %14lu %29s %pS\n",
name, stats->contention_point[i],
- ip, (void *)class->contention_point[i]);
+ ip, (void *)data->contention_point[i]);
}
for (i = 0; i < LOCKSTAT_POINTS; i++) {
char ip[32];
- if (class->contending_point[i] == 0)
+ if (data->contending_point[i] == 0)
break;
if (!i)
seq_line(m, '-', 40-namelen, namelen);
snprintf(ip, sizeof(ip), "[<%p>]",
- (void *)class->contending_point[i]);
+ (void *)data->contending_point[i]);
seq_printf(m, "%40s %14lu %29s %pS\n",
name, stats->contending_point[i],
- ip, (void *)class->contending_point[i]);
+ ip, (void *)data->contending_point[i]);
}
if (i) {
seq_puts(m, "\n");
@@ -593,6 +568,44 @@ static const struct seq_operations lockstat_ops = {
.show = ls_show,
};
+static void copy_lock_class(struct lock_stat_data *data,
+ struct lock_class *class)
+{
+ char *name = data->name;
+ int namelen = 38;
+
+ if (class->name_version > 1)
+ namelen -= 2; /* XXX truncates versions > 9 */
+ if (class->subclass)
+ namelen -= 2;
+
+ if (!class->name) {
+ char str[KSYM_NAME_LEN];
+ const char *key_name;
+
+ key_name = __get_key_name(class->key, str);
+ snprintf(name, namelen, "%s", key_name);
+ } else {
+ snprintf(name, namelen, "%s", class->name);
+ }
+ namelen = strlen(name);
+ if (class->name_version > 1) {
+ snprintf(name+namelen, 3, "#%d", class->name_version);
+ namelen += 2;
+ }
+ if (class->subclass) {
+ snprintf(name+namelen, 3, "/%d", class->subclass);
+ namelen += 2;
+ }
+
+ memcpy(data->contention_point, class->contention_point,
+ sizeof(class->contention_point));
+ memcpy(data->contending_point, class->contending_point,
+ sizeof(class->contending_point));
+
+ data->stats = lock_stats(class);
+}
+
static int lock_stat_open(struct inode *inode, struct file *file)
{
int res;
@@ -608,8 +621,7 @@ static int lock_stat_open(struct inode *inode, struct file *file)
struct seq_file *m = file->private_data;
list_for_each_entry(class, &all_lock_classes, lock_entry) {
- iter->class = class;
- iter->stats = lock_stats(class);
+ copy_lock_class(iter, class);
iter++;
}
data->iter_end = iter;
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists