Commit 8e56790b authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[PATCH] Fix race condition with current->group_info

From: Olaf Kirch <okir@suse.de>

I have been chasing a corruption of current->group_info on PPC during NFS
stress tests.  The problem seems to be that nfsd is messing with its
group_info quite a bit, while some monitoring processes look at
/proc/<pid>/status and do a get_group_info/put_group_info without any locking.

This problem can be reproduced on ppc platforms within a few seconds if you
generate some NFS load and do a "cat /proc/XXX/status" of an nfsd thread in a
tight loop.

I therefore think changes to current->group_info, and querying it from a
different process, needs to be protected using the task_lock.

(akpm: task->group_info here is safe against exit() because the task holds a
ref on group_info which is released in __put_task_struct, and the /proc file
has a ref on the task_struct).
parent af57e8ae
......@@ -149,6 +149,7 @@ static inline const char * get_task_state(struct task_struct *tsk)
static inline char * task_state(struct task_struct *p, char *buffer)
{
struct group_info *group_info;
int g;
read_lock(&tasklist_lock);
......@@ -174,12 +175,14 @@ static inline char * task_state(struct task_struct *p, char *buffer)
"FDSize:\t%d\n"
"Groups:\t",
p->files ? p->files->max_fds : 0);
group_info = p->group_info;
get_group_info(group_info);
task_unlock(p);
get_group_info(p->group_info);
for (g = 0; g < min(p->group_info->ngroups,NGROUPS_SMALL); g++)
buffer += sprintf(buffer, "%d ", GROUP_AT(p->group_info,g));
put_group_info(p->group_info);
for (g = 0; g < min(group_info->ngroups,NGROUPS_SMALL); g++)
buffer += sprintf(buffer, "%d ", GROUP_AT(group_info,g));
put_group_info(group_info);
buffer += sprintf(buffer, "\n");
return buffer;
......
......@@ -1281,8 +1281,12 @@ int set_current_groups(struct group_info *group_info)
groups_sort(group_info);
get_group_info(group_info);
task_lock(current);
old_info = current->group_info;
current->group_info = group_info;
task_unlock(current);
put_group_info(old_info);
return 0;
......@@ -1302,6 +1306,7 @@ asmlinkage long sys_getgroups(int gidsetsize, gid_t __user *grouplist)
if (gidsetsize < 0)
return -EINVAL;
/* no need to grab task_lock here; it cannot change */
get_group_info(current->group_info);
i = current->group_info->ngroups;
if (gidsetsize) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment