linux-kernel - Re: [PATCH] fixed spinlock use in hysdn_log

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]

Date:	Fri, 20 Apr 2007 17:41:46 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Matthias Kaehlcke <matthias.kaehlcke@...il.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	kkeil@...e.de, kai.germaschewski@....de,
	isdn4linux@...tserv.isdn4linux.de
Subject: Re: [PATCH] fixed spinlock use in hysdn_log_close()

On Sat, 14 Apr 2007 07:09:07 +0200
Matthias Kaehlcke <matthias.kaehlcke@...il.com> wrote:

> fixed incorrect spinlock use in hysdn_log_close(). the function
> declared a spinlock on the stack and used it to 'protect' a shared
> driver structure. the patch removes the declaration of hysdn_lock and 
> uses card->hysdn_lock instead.
> 

Interesting.

> 
> ---
> diff --git a/drivers/isdn/hysdn/hysdn_proclog.c b/drivers/isdn/hysdn/hysdn_proclog.c
> index f7e83a8..32f0b75 100644
> --- a/drivers/isdn/hysdn/hysdn_proclog.c
> +++ b/drivers/isdn/hysdn/hysdn_proclog.c
> @@ -299,7 +299,6 @@ hysdn_log_close(struct inode *ino, struct file *filep)
>  	hysdn_card *card;
>  	int retval = 0;
>  	unsigned long flags;
> -	spinlock_t hysdn_lock = SPIN_LOCK_UNLOCKED;
>  
>  	lock_kernel();
>  	if ((filep->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_WRITE) {
> @@ -309,7 +308,7 @@ hysdn_log_close(struct inode *ino, struct file *filep)
>  		/* read access -> log/debug read, mark one further file as closed */
>  
>  		pd = NULL;
> -		spin_lock_irqsave(&hysdn_lock, flags);
> +		spin_lock_irqsave(&card->hysdn_lock, flags);

I guess it won't hurt - are you actually able to test this code?

afaict most of the data in there is locked with lock_kernel(), if it's
locked at all.

If you had some runtime problem and this patch fixed it then fine.  If
however you're not able to test this code then perhaps the safest option is
to simply remove that locking altogether, which is pretty much a
runtime-equivalent change.

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