lkml.org 
[lkml]   [2015]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] fix a race between /proc/lock_stat and module unloading
Date
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@redhat.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


\
 
 \ /
  Last update: 2015-05-29 15:21    [W:0.059 / U:8.880 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site