Messages in this thread |  | | | Date | Fri, 2 Jul 2010 14:14:56 +0200 | | From | Jiri Olsa <> | | Subject | Re: [PATCH] cred - synchronize rcu before releasing cred |
| |
hi, any feedback?
jirka
On Fri, Jun 25, 2010 at 09:33:33AM -0400, Jiri Olsa wrote: > BZ 591015 - kernel BUG at kernel/cred.c:168 > https://bugzilla.redhat.com/show_bug.cgi?id=591015 > > Above bugzilla reported bug during the releasing of > old cred structure. > > There is reproducer attached to the bugzilla. > > The issue is caused by releasing old cred struct while other > kernel path might be still using it. This leads to cred->usage > inconsistency inside the __put_cred and triggering the bug. > > Following kernel paths are affected: > > The CPU1 path is setting the new groups creds. > The CPU2 path is cat /proc/PID/status > > > CPU 1 CPU 2 > > sys_setgroups proc_pid_status > set_current_groups task_state > commit_creds rcu_read_lock > put_cred ... > __put_cred get_cred > BUG_ON(usage != 0) ... > rcu_read_unlock > > > > If __put_cred got executed during the CPU2 holding the reference > the BUG_ON inside __put_cred is trigered. > > I think there's no need to get the cred refference as long as > the 'cred' handling stays inside the rcu_read_lock block. > > And the condition of __task_cred 'make sure task doesn't go away', > is done by proc_single_show as this is the proc file. > > wbr, > jirka > > > Signed-off-by: Jiri Olsa <jolsa@redhat.com> > Acked-by: David Howells <dhowells@redhat.com> > --- > diff --git a/fs/proc/array.c b/fs/proc/array.c > index 9b58d38..ac3b3a4 100644 > --- a/fs/proc/array.c > +++ b/fs/proc/array.c > @@ -176,7 +176,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, > if (tracer) > tpid = task_pid_nr_ns(tracer, ns); > } > - cred = get_cred((struct cred *) __task_cred(p)); > + cred = __task_cred(p); > seq_printf(m, > "State:\t%s\n" > "Tgid:\t%d\n" > @@ -199,15 +199,14 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, > "FDSize:\t%d\n" > "Groups:\t", > fdt ? fdt->max_fds : 0); > - rcu_read_unlock(); > > group_info = cred->group_info; > task_unlock(p); > > for (g = 0; g < min(group_info->ngroups, NGROUPS_SMALL); g++) > seq_printf(m, "%d ", GROUP_AT(group_info, g)); > - put_cred(cred); > > + rcu_read_unlock(); > seq_printf(m, "\n"); > } >
|  |