linux-kernel - Re: [PATCH 1/3] staging:panel: Rewrite for fixing synchronization issues

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, 29 Jun 2012 10:24:41 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Zoltan Kelemen <zoltan@...isec.se>
Cc:	willy@...a-x.org, gregkh@...uxfoundation.org,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] staging:panel: Rewrite for fixing synchronization
 issues

On Thu, Jun 28, 2012 at 10:57:29PM +0200, Zoltan Kelemen wrote:

> Partial rewrite of panel driver to make it more robust. The old code contains
> problems potentially leading to kernel oops, deadlocks and memory leaks.
> 
> The patch contains the following changes:
> * Handle asynchronous attach/detach to/from parport.
> * Proper synchronization of multiple opens to same device.
> * Proper disabling of softirqs when locking pprt from user context.
> * Replaced interruptible_sleep_on in keypad_read to avoid race conditions.
> * Changed to del_timer_sync to make sure timer function finishes before
>   unloading module.
> * Added owner to file_operations to avoid module being unloaded with dangling
>   opens.
> * Check status of user-mode memory write in keypad_read.
> * Check for and handle errors in keypad_init.
> * Properly free memory for logical_inputs to avoid memory leak on unload.
> 

Gar...  Don't do this.  :/  Each bullet point here should be a
separate patch.  Please break it apart and resend.

Put the white space changes and the declaration reshuffles in their
own patch.

Do not create lock_pprt() and unlock_pprt() functions.  It doesn't
save any lines of code.  It means that Sparse is unable check for
locking bugs.  It hides information pointlessly so it's considered
bad style.  The comment is nice though and that should go next to
the declaration of the "pprt_lock" spinlock.

regards,
dan carpenter


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