netdev - Re: [RCU PATCH 06/14] net: sched: fw use RCU

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:	Thu, 13 Mar 2014 13:22:46 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	John Fastabend <john.fastabend@...il.com>,
	xiyou.wangcong@...il.com, jhs@...atatu.com, netdev@...r.kernel.org,
	davem@...emloft.net
Subject: Re: [RCU PATCH 06/14] net: sched: fw use RCU

On Wed, Mar 12, 2014 at 10:01:13AM -0700, Eric Dumazet wrote:
> On Wed, 2014-03-12 at 09:41 -0700, John Fastabend wrote:
> 
> > Rearranging the rcu_head placement above results in this error:
> > 
> > net/sched/cls_fw.c: In function ‘fw_destroy’:
> > net/sched/cls_fw.c:169:279: error: call to ‘__compiletime_assert_169’ 
> > declared with attribute error: BUILD_BUG_ON failed: 
> > !__is_kfree_rcu_offset(__builtin_offsetof(typeof(*(head)),rcu))
> > 
> > 
> > However it seems best to have the rcu_head at the end of the struct
> > so I'll just convert the kfree_rcu() to call_rcu().
> 
> 
> I missed HTSIZE was so big.
> 
> Its unfortunate because it makes struct fw_head slightly larger than one
> page.

Hmmm...  You know, the offset that kfree uses is a byte offset.  I see
no reason why any of the rcu_head structures should be misaligned.  If
HTSIZE is too big by only a factor of four or smaller, would the following
(untested) patch to RCU be appropriate?

							Thanx, Paul

------------------------------------------------------------------------

rcu: Increase kfree_rcu() offset range

The kfree_rcu() function operates by placing an offset into the rcu_head
structure in place of the function pointer that is normally there.
This offset cannot exceed 4095, on the theory that no Linux kernel will
ever have executable code in page zero.  However, the rcu_head structure
contains a pair of function pointers, and should never be misaligned.
This commit therefore divides the offset by the size of a pointer when
being placed into the rcu_head structure, and multiplies it by this
same constant when pulling it out.

Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 3cea28c64ebe..2dc4ecf923d0 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -1023,8 +1023,13 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
  * The BUILD_BUG_ON check must not involve any function calls, hence the
  * checks are done in macros here.
  */
-#define kfree_rcu(ptr, rcu_head)					\
-	__kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
+#define kfree_rcu(ptr, rcu_head) \
+do { \
+	unsigned long ___offset = offsetof(typeof(*(ptr)), rcu_head); \
+	\
+	BUILD_BUG_ON(___offset & (sizeof(void *) - 1)); \
+	__kfree_rcu(&((ptr)->rcu_head), ___offset / sizeof(void *)); \
+} while (0)
 
 #if defined(CONFIG_TINY_RCU) || defined(CONFIG_RCU_NOCB_CPU_ALL)
 static inline int rcu_needs_cpu(int cpu, unsigned long *delta_jiffies)
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index bfda2726ca45..93dbd98ad6da 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -106,7 +106,7 @@ static inline bool __rcu_reclaim(const char *rn, struct rcu_head *head)
 	rcu_lock_acquire(&rcu_callback_map);
 	if (__is_kfree_rcu_offset(offset)) {
 		RCU_TRACE(trace_rcu_invoke_kfree_callback(rn, head, offset));
-		kfree((void *)head - offset);
+		kfree((void *)head - offset * sizeof(void *));
 		rcu_lock_release(&rcu_callback_map);
 		return 1;
 	} else {

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html