linux-kernel - Re: [srcu] Can we suppress sparse warning?

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: <201611132155.BCA46603.HOVtQSFOOFMLFJ@I-love.SAKURA.ne.jp>
Date:   Sun, 13 Nov 2016 21:55:10 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     paulmck@...ux.vnet.ibm.com
Cc:     josh@...htriplett.org, rostedt@...dmis.org,
        mathieu.desnoyers@...icios.com, jiangshanlai@...il.com,
        linux-kernel@...r.kernel.org
Subject: Re: [srcu] Can we suppress sparse warning?

Paul E. McKenney wrote:
> On Sat, Nov 12, 2016 at 11:26:15PM +0900, Tetsuo Handa wrote:
> > Both head->r.acl and &tomoyo_kernel_namespace.policy_list[TOMOYO_ID_MANAGER]->next
> > refer normal kernel address space. Thus, I think that this warning is a false positive.
> > 
> > This warning goes away if I disable rcu_dereference_sparse() call in
> > __rcu_dereference_check() from srcu_dereference_check() from srcu_dereference().
> > 
> > --- a/include/linux/rcupdate.h
> > +++ b/include/linux/rcupdate.h
> > @@ -605,7 +605,7 @@ static inline void rcu_preempt_sleep_check(void)
> >         /* Dependency order vs. p above. */ \
> >         typeof(*p) *________p1 = (typeof(*p) *__force)lockless_dereference(p); \
> >         RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
> > -       rcu_dereference_sparse(p, space); \
> > +       /*rcu_dereference_sparse(p, space); */                          \
> >         ((typeof(*p) __force __kernel *)(________p1)); \
> >  })
> >  #define __rcu_dereference_protected(p, c, space) \
> 
> OK.
> 
> One approach would be for you to replace your:
> 
> 	pos =  srcu_dereference((head)->next, &tomoyo_ss);
> 
> with:
> 
> 	pos =  ecu_dereference_raw((head)->next);
> 
> This will suppress the sparse complaint, but would also suppress the
> lockdep complaint about using list_for_each_cookie() unprotected by an
> SRCU read-side critical section.  But this can be handled by placing an
> appropriate RCU_LOCKDEP_WARN() in list_for_each_cookie().
> 
> Does that work for you?

Yes, I can use below approach. Thank you.

--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -1317,14 +1317,20 @@ static inline int tomoyo_round2(size_t size)
 
 #endif
 
+static inline void tomoyo_read_lock_check(void)
+{
+	RCU_LOCKDEP_WARN(!srcu_read_lock_held(&tomoyo_ss),
+			 "tomoyo_ss lock is not held.");
+}
+
 /**
  * list_for_each_cookie - iterate over a list with cookie.
  * @pos:        the &struct list_head to use as a loop cursor.
  * @head:       the head for your list.
  */
 #define list_for_each_cookie(pos, head)					\
-	if (!pos)							\
-		pos =  srcu_dereference((head)->next, &tomoyo_ss);	\
-	for ( ; pos != (head); pos = srcu_dereference(pos->next, &tomoyo_ss))
+	for (pos = pos ? pos : rcu_dereference_raw((head)->next);	\
+	     tomoyo_read_lock_check(), pos != (head);			\
+	     pos = rcu_dereference_raw(pos->next))
 
 #endif /* !defined(_SECURITY_TOMOYO_COMMON_H) */