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  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:   Mon, 17 Feb 2020 17:08:27 +0100
From:   Uladzislau Rezki <urezki@...il.com>
To:     "Paul E. McKenney" <paulmck@...nel.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        "Theodore Y. Ts'o" <tytso@....edu>
Cc:     "Theodore Y. Ts'o" <tytso@....edu>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Suraj Jitindar Singh <surajjs@...zon.com>,
        Uladzislau Rezki <urezki@...il.com>,
        Joel Fernandes <joel@...lfernandes.org>
Subject: Re: [PATCH RFC] ext4: fix potential race between online resizing and
 write operations

Hello, Joel, Paul, Ted. 

> 
> Good point!
> 
> Now that kfree_rcu() is on its way to being less of a hack deeply
> entangled into the bowels of RCU, this might be fairly easy to implement.
> It might well be simply a matter of a function pointer and a kvfree_rcu()
> API.  Adding Uladzislau Rezki and Joel Fernandez on CC for their thoughts.
> 
I think it makes sense. For example i see there is a similar demand in
the mm/list_lru.c too. As for implementation, it will not be hard, i think. 

The easiest way is to inject kvfree() support directly into existing kfree_call_rcu()
logic(probably will need to rename that function), i.e. to free vmalloc() allocations
only in "emergency path" by just calling kvfree(). So that function in its turn will
figure out if it is _vmalloc_ address or not and trigger corresponding "free" path.

Just an example:

<snip>
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 75a2eded7aa2..0c4af5d0e3f8 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -806,6 +806,11 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
                kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
        } while (0)

+#define __kvfree_rcu(head, offset) \
+       do { \
+               kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
+       } while (0)
+
 /**
  * kfree_rcu() - kfree an object after a grace period.
  * @ptr:       pointer to kfree
@@ -840,6 +845,14 @@ do {                                                                       \
                __kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
 } while (0)

+#define kvfree_rcu_my(ptr, rhf)                                                \
+do {                                                                   \
+       typeof (ptr) ___p = (ptr);                                      \
+                                                                       \
+       if (___p)                                                       \
+               __kvfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
+} while (0)
+
 /*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 394a83bd7ff4..1a05bb44951b 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2783,11 +2783,10 @@ static void kfree_rcu_work(struct work_struct *work)
                rcu_lock_acquire(&rcu_callback_map);
                trace_rcu_invoke_kfree_callback(rcu_state.name, head, offset);
 
-               if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset)))
-                       kfree((void *)head - offset);
+               kvfree((void *)head - offset);
 
-                rcu_lock_release(&rcu_callback_map);
-                cond_resched_tasks_rcu_qs();
+               rcu_lock_release(&rcu_callback_map);
+               cond_resched_tasks_rcu_qs();
        }
 }
 
@@ -2964,7 +2963,8 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
         * Under high memory pressure GFP_NOWAIT can fail,
         * in that case the emergency path is maintained.
         */
-       if (unlikely(!kfree_call_rcu_add_ptr_to_bulk(krcp, head, func))) {
+       if (is_vmalloc_addr((void *) head - (unsigned long) func) ||
+                       unlikely(!kfree_call_rcu_add_ptr_to_bulk(krcp, head, func))) {
                head->func = func;
                head->next = krcp->head;
                krcp->head = head;
<snip>

How to use it:

<snip>
struct test_kvfree_rcu {
       unsigned char array[PAGE_SIZE * 2];
       struct rcu_head rcu;
};

struct test_kvfree_rcu *p;

p = vmalloc(sizeof(struct test_kvfree_rcu));
kvfree_rcu_my((struct test_kvfree_rcu *) p, rcu);
<snip>

Any thoughts?

Thank you!

--
Vlad Rezki

Powered by blists - more mailing lists