linux-kernel - Re: [PATCH] sparse fix: add many lock annotations

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] [day] [month] [year] [list]

Message-ID: <20061124184758.GA4973@martell.zuzino.mipt.ru>
Date:	Fri, 24 Nov 2006 21:47:59 +0300
From:	Alexey Dobriyan <adobriyan@...il.com>
To:	Ira Snyder <kernel@...snyder.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sparse fix: add many lock annotations

On Thu, Nov 23, 2006 at 12:18:42AM -0800, Ira Snyder wrote:
> On Wed, 22 Nov 2006 21:33:07 +0300
> Alexey Dobriyan <adobriyan@...il.com> wrote:
> > On Wed, Nov 22, 2006 at 12:11:46AM -0800, Ira Snyder wrote:
> > > This patch adds many lock annotations to the kernel source to quiet
> > > warnings from sparse. In almost every case, it quiets the warning caused
> > > by locks that are intentionally grabbed in one function and released in
> > > another.
> > >
> > > In the other cases, __acquire() and __release() are used to make sparse
> > > believe that a lock was grabbed (even though it was not), in order to
> > > make all exit points have equal lock counts. These follow the style in
> > > kernel/sched.c.
> >
> > > --- a/arch/i386/kernel/smp.c
> > > +++ b/arch/i386/kernel/smp.c
> > > @@ -507,11 +507,13 @@ struct call_data_struct {
> > >  };
> > >
> > >  void lock_ipi_call_lock(void)
> > > +__acquires(call_lock)
> > >  {
> > >  	spin_lock_irq(&call_lock);
> > >  }
> > >
> > >  void unlock_ipi_call_lock(void)
> > > +__releases(call_lock)
> > >  {
> > >  	spin_unlock_irq(&call_lock);
> > >  }
> >
> > Wrong place. Prototypes should be marked instead. How else would you
> > know about:
> >
> > 	lock_ipi_call_lock();
> > 	if (foo)
> > 		return -E;
> > 	lock_ipi_call_lock();
> >
> > on another compilation unit?
> >
>
> I've re-thought about this since my last email, and I see what you are
> saying now. Functions which are static shouldn't need the __releases()
> or __acquires() anywhere except the definition, since they cannot be
> used outside of the file in which they reside. Non-static functions do
> need to be marked in the prototypes (so all external uses see the
> marking, and therefore get the benefit of sparse's checking) as well as
> in the definition (to quiet sparse itself).

Yes, that's it.

-
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/