Prev: firewire: ohci: use memory barriers to order descriptor updates
Next: [PATCH] [resend] cyber2000fb: add mode_option parameter
From: Jiri Olsa on 27 Jul 2010 08:20
hi,
any feedback?

jirka

On Fri, Jul 02, 2010 at 02:14:56PM +0200, Jiri Olsa wrote:
> 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(a)redhat.com>
> > Acked-by: David Howells <dhowells(a)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");
> > }
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Howells on 27 Jul 2010 08:50
Jiri Olsa <jolsa(a)redhat.com> wrote:

> any feedback?

Guess no one's objecting. Post it to Linus then.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: firewire: ohci: use memory barriers to order descriptor updates
Next: [PATCH] [resend] cyber2000fb: add mode_option parameter