Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge tag 'probes-fixes-v6.7-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace

Pull probes fixes from Masami Hiramatsu:

- objpool: Fix objpool overrun case on memory/cache access delay
especially on the big.LITTLE SoC. The objpool uses a copy of object
slot index internal loop, but the slot index can be changed on
another processor in parallel. In that case, the difference of 'head'
local copy and the 'slot->last' index will be bigger than local slot
size. In that case, we need to re-read the slot::head to update it.

- kretprobe: Fix to use appropriate rcu API for kretprobe holder. Since
kretprobe_holder::rp is RCU managed, it should use
rcu_assign_pointer() and rcu_dereference_check() correctly. Also
adding __rcu tag for finding wrong usage by sparse.

- rethook: Fix to use appropriate rcu API for rethook::handler. The
same as kretprobe, rethook::handler is RCU managed and it should use
rcu_assign_pointer() and rcu_dereference_check(). This also adds
__rcu tag for finding wrong usage by sparse.

* tag 'probes-fixes-v6.7-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace:
rethook: Use __rcu pointer for rethook::handler
kprobes: consistent rcu api usage for kretprobe holder
lib: objpool: fix head overrun on RK3588 SBC

+43 -21
+4 -9
include/linux/kprobes.h
··· 139 139 * 140 140 */ 141 141 struct kretprobe_holder { 142 - struct kretprobe *rp; 142 + struct kretprobe __rcu *rp; 143 143 struct objpool_head pool; 144 144 }; 145 145 ··· 197 197 #ifdef CONFIG_KRETPROBE_ON_RETHOOK 198 198 static nokprobe_inline struct kretprobe *get_kretprobe(struct kretprobe_instance *ri) 199 199 { 200 - RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(), 201 - "Kretprobe is accessed from instance under preemptive context"); 202 - 203 - return (struct kretprobe *)READ_ONCE(ri->node.rethook->data); 200 + /* rethook::data is non-changed field, so that you can access it freely. */ 201 + return (struct kretprobe *)ri->node.rethook->data; 204 202 } 205 203 static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_instance *ri) 206 204 { ··· 243 245 244 246 static nokprobe_inline struct kretprobe *get_kretprobe(struct kretprobe_instance *ri) 245 247 { 246 - RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(), 247 - "Kretprobe is accessed from instance under preemptive context"); 248 - 249 - return READ_ONCE(ri->rph->rp); 248 + return rcu_dereference_check(ri->rph->rp, rcu_read_lock_any_held()); 250 249 } 251 250 252 251 static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_instance *ri)
+6 -1
include/linux/rethook.h
··· 28 28 */ 29 29 struct rethook { 30 30 void *data; 31 - rethook_handler_t handler; 31 + /* 32 + * To avoid sparse warnings, this uses a raw function pointer with 33 + * __rcu, instead of rethook_handler_t. But this must be same as 34 + * rethook_handler_t. 35 + */ 36 + void (__rcu *handler) (struct rethook_node *, void *, unsigned long, struct pt_regs *); 32 37 struct objpool_head pool; 33 38 struct rcu_head rcu; 34 39 };
+2 -2
kernel/kprobes.c
··· 2252 2252 rp->rph = NULL; 2253 2253 return -ENOMEM; 2254 2254 } 2255 - rp->rph->rp = rp; 2255 + rcu_assign_pointer(rp->rph->rp, rp); 2256 2256 rp->nmissed = 0; 2257 2257 /* Establish function entry probe point */ 2258 2258 ret = register_kprobe(&rp->kp); ··· 2300 2300 #ifdef CONFIG_KRETPROBE_ON_RETHOOK 2301 2301 rethook_free(rps[i]->rh); 2302 2302 #else 2303 - rps[i]->rph->rp = NULL; 2303 + rcu_assign_pointer(rps[i]->rph->rp, NULL); 2304 2304 #endif 2305 2305 } 2306 2306 mutex_unlock(&kprobe_mutex);
+14 -9
kernel/trace/rethook.c
··· 48 48 */ 49 49 void rethook_stop(struct rethook *rh) 50 50 { 51 - WRITE_ONCE(rh->handler, NULL); 51 + rcu_assign_pointer(rh->handler, NULL); 52 52 } 53 53 54 54 /** ··· 63 63 */ 64 64 void rethook_free(struct rethook *rh) 65 65 { 66 - WRITE_ONCE(rh->handler, NULL); 66 + rethook_stop(rh); 67 67 68 68 call_rcu(&rh->rcu, rethook_free_rcu); 69 69 } ··· 80 80 { 81 81 kfree(context); 82 82 return 0; 83 + } 84 + 85 + static inline rethook_handler_t rethook_get_handler(struct rethook *rh) 86 + { 87 + return (rethook_handler_t)rcu_dereference_check(rh->handler, 88 + rcu_read_lock_any_held()); 83 89 } 84 90 85 91 /** ··· 113 107 return ERR_PTR(-ENOMEM); 114 108 115 109 rh->data = data; 116 - rh->handler = handler; 110 + rcu_assign_pointer(rh->handler, handler); 117 111 118 112 /* initialize the objpool for rethook nodes */ 119 113 if (objpool_init(&rh->pool, num, size, GFP_KERNEL, rh, ··· 141 135 */ 142 136 void rethook_recycle(struct rethook_node *node) 143 137 { 144 - lockdep_assert_preemption_disabled(); 138 + rethook_handler_t handler; 145 139 146 - if (likely(READ_ONCE(node->rethook->handler))) 140 + handler = rethook_get_handler(node->rethook); 141 + if (likely(handler)) 147 142 objpool_push(node, &node->rethook->pool); 148 143 else 149 144 call_rcu(&node->rcu, free_rethook_node_rcu); ··· 160 153 */ 161 154 struct rethook_node *rethook_try_get(struct rethook *rh) 162 155 { 163 - rethook_handler_t handler = READ_ONCE(rh->handler); 164 - 165 - lockdep_assert_preemption_disabled(); 156 + rethook_handler_t handler = rethook_get_handler(rh); 166 157 167 158 /* Check whether @rh is going to be freed. */ 168 159 if (unlikely(!handler)) ··· 305 300 rhn = container_of(first, struct rethook_node, llist); 306 301 if (WARN_ON_ONCE(rhn->frame != frame)) 307 302 break; 308 - handler = READ_ONCE(rhn->rethook->handler); 303 + handler = rethook_get_handler(rhn->rethook); 309 304 if (handler) 310 305 handler(rhn, rhn->rethook->data, 311 306 correct_ret_addr, regs);
+17
lib/objpool.c
··· 201 201 while (head != READ_ONCE(slot->last)) { 202 202 void *obj; 203 203 204 + /* 205 + * data visibility of 'last' and 'head' could be out of 206 + * order since memory updating of 'last' and 'head' are 207 + * performed in push() and pop() independently 208 + * 209 + * before any retrieving attempts, pop() must guarantee 210 + * 'last' is behind 'head', that is to say, there must 211 + * be available objects in slot, which could be ensured 212 + * by condition 'last != head && last - head <= nr_objs' 213 + * that is equivalent to 'last - head - 1 < nr_objs' as 214 + * 'last' and 'head' are both unsigned int32 215 + */ 216 + if (READ_ONCE(slot->last) - head - 1 >= pool->nr_objs) { 217 + head = READ_ONCE(slot->head); 218 + continue; 219 + } 220 + 204 221 /* obj must be retrieved before moving forward head */ 205 222 obj = READ_ONCE(slot->entries[head & slot->mask]); 206 223