Re: ABI change in fix for CVE-2008-5029

[dann frazier <dannf@debian.org>]

On Fri, Nov 14, 2008 at 10:23:22PM +0100, Bastian Blank wrote:
> On Fri, Nov 14, 2008 at 01:29:05PM -0700, dann frazier wrote:
> > Because this affects a significant number of symbols, it doesn't look
> > to me like a safe thing to ignore w/ the #ifdef __GENKSYMS__ trick, so
> > its looking like we need to increment the ABI for the stable kernels,
> > and perhaps the lenny kernel. Do others on the team have a different
> > opinion?
> 
> You did not dig deep enough.

Well, I came up with the same patch you did...

>  It is a change in the task_struct. As long
> as this struct is never allocated outside of the core kernel (doing so
> would be insane anyway), the following patch will do.

I suppose so; I just don't like the guesswork. But, there aren't any
in-tree examples of allocating task_struct outside of the core, so
I'll concede that its probably safe.

My 2.6.18 backport is a little more invasive - it includes a few
changes to struct unix_sock. It appears to be mostly an internal
structure as well, but it worries me more because we're not just
tacking on a new field to the end:

--- af_unix.orig	2008-11-14 16:09:04.000000000 -0700
+++ af_unix.h	2008-11-14 16:08:37.000000000 -0700
@@ -81,9 +81,11 @@
 	struct mutex		readlock;
         struct sock		*peer;
         struct sock		*other;
-        struct sock		*gc_tree;
+	struct list_head	link;
         atomic_t                inflight;
         spinlock_t		lock;
+	unsigned int		gc_candidate : 1;
+	unsigned int		gc_maybe_cycle : 1;
         wait_queue_head_t       peer_wait;
 };
 #define unix_sk(__sk) ((struct unix_sock *)__sk)

There's one example in 2.6.18 where this is allocated out of tree (in
selinux). I'll go ahead and commit what I've got in case you want
context for this hunk.

> | --- a/include/linux/sched.h
> | +++ b/include/linux/sched.h
> | @@ -1288,8 +1288,6 @@ struct task_struct {
> |         atomic_t fs_excl;       /* holding fs exclusive resources */
> |         struct rcu_head rcu;
> | 
> | -       struct list_head        *scm_work_list;
> | -
> |         /*
> |          * cache last used pipe for splice
> |          */
> | @@ -1305,6 +1303,10 @@ struct task_struct {
> |         int latency_record_count;
> |         struct latency_record latency_record[LT_SAVECOUNT];
> |  #endif
> | +
> | +#ifndef __GENKSYMS__
> | +       struct list_head        *scm_work_list;
> | +#endif
> |  };
> | 
> |  /*
> 
> However, there is a second change: scm_*, four or so. This symbols are
> only used inside the core (by the unix and netlink socket support), so I
> would ignore that.

*nod*

-- 
dann frazier