linux-kernel - Re: [TOMOYO 05/15](repost) Domain transition handler functions.

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: <Xine.LNX.4.64.0710020557170.21841@us.intercode.com.au>
Date:	Tue, 2 Oct 2007 06:07:08 -0700 (PDT)
From:	James Morris <jmorris@...ei.org>
To:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
cc:	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org, chrisw@...s-sol.org
Subject: Re: [TOMOYO 05/15](repost) Domain transition handler functions.

On Tue, 2 Oct 2007, Tetsuo Handa wrote:

> Hello.
> 
> Thank you for your comment.
> 
> James Morris wrote:
> > Why do you need to avoid spinlocks for these manipulations?
> 
> I don't need to use spinlocks here.
> What I need to do here is avoid read/write reordering,
> so mb() will be appropriate here.
> 
> struct something {
>   struct something *next;
>   void *data;
> };
> 
> struct something *prev_ptr = ...;
> struct something *new_ptr = kmalloc(sizeof(*new_ptr), GFP_KERNEL);
> new_ptr->next = NULL;
> new_ptr->data = some_value;
> mb();
> prev_ptr->next = new_ptr;

You're introducing a custom API, which is open-coded repeatedly throughout 
your module.

All linked lists (at least, new ones) must use the standard kernel list
API.

> Performance is not critical because this mb() is called
> only when appending new entry to the singly-linked list.

Most of these uses appear to be slow path or nested inside other locks, 
while overall, performance is likely to be dominated by your string 
matching and permission checking.  Use of mb(), which is typically 
considered less understandable, in this case does not appear to be 
justified vs. normal locking, and you also need to use the standard list 
API.  If performance really is an issue, then consider RCU.

- James
-- 
James Morris
<jmorris@...ei.org>
-
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/