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

Message-ID: <20061122183306.GA4970@martell.zuzino.mipt.ru>
Date:	Wed, 22 Nov 2006 21:33:07 +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 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?

> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1004,6 +1004,7 @@ EXPORT_SYMBOL(max_cstate);
>   */
>
>  acpi_cpu_flags acpi_os_acquire_lock(acpi_spinlock lockp)
> +__acquires(lockp)
>  {
>  	acpi_cpu_flags flags;
>  	spin_lock_irqsave(lockp, flags);
> @@ -1015,6 +1016,7 @@ acpi_cpu_flags acpi_os_acquire_lock(acpi
>   */
>
>  void acpi_os_release_lock(acpi_spinlock lockp, acpi_cpu_flags flags)
> +__releases(lockp)
>  {
>  	spin_unlock_irqrestore(lockp, flags);
>  }

Again, wrong. IMO, sparse should deduce itself that lock is grabbed in such
trivial cases.

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