linux-kernel - Re: [git pull] core kernel fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080630182038.GA32211@elte.hu>
Date:	Mon, 30 Jun 2008 20:20:38 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Vegard Nossum <vegard.nossum@...il.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Daniel J Blueman <daniel.blueman@...il.com>
Subject: Re: [git pull] core kernel fixes


* Vegard Nossum <vegard.nossum@...il.com> wrote:

> On Mon, Jun 30, 2008 at 5:32 PM, Ingo Molnar <mingo@...e.hu> wrote:
> > diff --git a/lib/debugobjects.c b/lib/debugobjects.c
> > index a76a5e1..85b18d7 100644
> > --- a/lib/debugobjects.c
> > +++ b/lib/debugobjects.c
> > @@ -68,6 +68,7 @@ static int fill_pool(void)
> >  {
> >        gfp_t gfp = GFP_ATOMIC | __GFP_NORETRY | __GFP_NOWARN;
> >        struct debug_obj *new;
> > +       unsigned long flags;
> >
> >        if (likely(obj_pool_free >= ODEBUG_POOL_MIN_LEVEL))
> >                return obj_pool_free;
> > @@ -81,10 +82,10 @@ static int fill_pool(void)
> >                if (!new)
> >                        return obj_pool_free;
> >
> > -               spin_lock(&pool_lock);
> > +               spin_lock_irqsave(&pool_lock, flags);
> >                hlist_add_head(&new->node, &obj_pool);
> >                obj_pool_free++;
> > -               spin_unlock(&pool_lock);
> > +               spin_unlock_irqrestore(&pool_lock, flags);
> >        }
> >        return obj_pool_free;
> >  }
> > @@ -110,16 +111,13 @@ static struct debug_obj *lookup_object(void *addr, struct debug_bucket *b)
> >  }
> >
> >  /*
> > - * Allocate a new object. If the pool is empty and no refill possible,
> > - * switch off the debugger.
> > + * Allocate a new object. If the pool is empty, switch off the debugger.
> >  */
> >  static struct debug_obj *
> >  alloc_object(void *addr, struct debug_bucket *b, struct debug_obj_descr *descr)
> >  {
> >        struct debug_obj *obj = NULL;
> > -       int retry = 0;
> >
> > -repeat:
> >        spin_lock(&pool_lock);
> >        if (obj_pool.first) {
> >                obj         = hlist_entry(obj_pool.first, typeof(*obj), node);
> > @@ -141,9 +139,6 @@ repeat:
> >        }
> >        spin_unlock(&pool_lock);
> >
> > -       if (fill_pool() && !obj && !retry++)
> > -               goto repeat;
> > -
> >        return obj;
> >  }
> >
> > @@ -261,6 +256,8 @@ __debug_object_init(void *addr, struct debug_obj_descr *descr, int onstack)
> >        struct debug_obj *obj;
> >        unsigned long flags;
> >
> > +       fill_pool();
> > +
> >        db = get_bucket((unsigned long) addr);
> >
> >        spin_lock_irqsave(&db->lock, flags);
> > --
> 
> Hm. I have to wonder where this patch came from.
> 
> This was my (faulty) patch: http://lkml.org/lkml/2008/6/14/193
> and Daniel J Blueman followed up with this: http://lkml.org/lkml/2008/6/15/27
> 
> ..but this one looks different from both. I am guessing the last bits 
> were added (or removed?) by Thomas?
> 
> I am wondering if the final patch was tested with the reproducible 
> test case (if so, by whom?) and whether should be credited to Daniel 
> (or Thomas?) instead...

You can use "git log -1 -p --pretty=fuller 50db04dd9c" to see the exact 
details of the commit:

----------
| commit 50db04dd9c74178e68a981a7127c37252ffb3242
| Author:     Vegard Nossum <vegard.nossum@...il.com>
| AuthorDate: Sun Jun 15 00:47:36 2008 +0200
| Commit:     Thomas Gleixner <tglx@...utronix.de>
| CommitDate: Wed Jun 18 11:09:54 2008 +0200
|
| [...]
|
|   [ daniel.blueman@...il.com: pool_lock needs to be taken irq safe in fill_pool ]
|
|   Reported-by: Daniel J Blueman <daniel.blueman@...il.com>
|   Signed-off-by: Vegard Nossum <vegard.nossum@...il.com>
|   Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
----------

As you can see it from the Commit line, it was committed by Thomas.

The "[ daniel.blueman: pool_lock ... ]" line shows that Thomas - instead 
of creating two commits - merged the two fixes into a single commit and 
credited Daniel for the irq-safe fix. This is the standard technique to 
squash small patches and to make fixes multi-authored.

The patch was tested with our standard tests so it's certainly good in 
practice - but i havent specifically tried your testcase (maybe Thomas 
has). Can you see any problem with the fix?

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/