When list_for_each_entry() completes the iteration over the whole list without breaking the loop, the iterator value will be a bogus pointer computed based on the head element. While it is safe to use the pointer to determine if it was computed based on the head element, either with list_entry_is_head() or &pos->member == head, using the iterator variable after the loop should be avoided. In preparation to limit the scope of a list iterator to the list traversal loop, use a dedicated pointer to point to the found element [1]. Link: https://lkml.kernel.org/r/20220427170734.819891-5-jakobkoschel@gmail.com
Showing the changes to kernel (3 of 3 files). Show all
Unified · Split
| @@ -4560,8 +4560,8 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr, · 4559 unmodified lines | |||
| 4560 | struct ftrace_probe_ops *probe_ops, | 4560 | struct ftrace_probe_ops *probe_ops, |
| 4561 | void *data) | 4561 | void *data) |
| 4562 | { | 4562 | { |
| 4563 | struct ftrace_func_probe *probe = NULL, *iter; | ||
| 4563 | struct ftrace_func_entry *entry; | 4564 | struct ftrace_func_entry *entry; |
| 4564 | struct ftrace_func_probe *probe; | ||
| 4565 | struct ftrace_hash **orig_hash; | 4565 | struct ftrace_hash **orig_hash; |
| 4566 | struct ftrace_hash *old_hash; | 4566 | struct ftrace_hash *old_hash; |
| 4567 | struct ftrace_hash *hash; | 4567 | struct ftrace_hash *hash; |
| @@ -4580,11 +4580,13 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr, · 12 unmodified lines | |||
| 4580 | 4580 | ||
| 4581 | mutex_lock(&ftrace_lock); | 4581 | mutex_lock(&ftrace_lock); |
| 4582 | /* Check if the probe_ops is already registered */ | 4582 | /* Check if the probe_ops is already registered */ |
| 4583 | list_for_each_entry(probe, &tr->func_probes, list) { | 4583 | list_for_each_entry(iter, &tr->func_probes, list) { |
| 4584 | if (probe->probe_ops == probe_ops) | 4584 | if (iter->probe_ops == probe_ops) { |
| 4585 | probe = iter; | ||
| 4585 | break; | 4586 | break; |
| 4587 | } | ||
| 4586 | } | 4588 | } |
| 4587 | if (&probe->list == &tr->func_probes) { | 4589 | if (!probe) { |
| 4588 | probe = kzalloc(sizeof(*probe), GFP_KERNEL); | 4590 | probe = kzalloc(sizeof(*probe), GFP_KERNEL); |
| 4589 | if (!probe) { | 4591 | if (!probe) { |
| 4590 | mutex_unlock(&ftrace_lock); | 4592 | mutex_unlock(&ftrace_lock); |
| @@ -4702,9 +4704,9 @@ int · 111 unmodified lines | |||
| 4702 | unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr, | 4704 | unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr, |
| 4703 | struct ftrace_probe_ops *probe_ops) | 4705 | struct ftrace_probe_ops *probe_ops) |
| 4704 | { | 4706 | { |
| 4707 | struct ftrace_func_probe *probe = NULL, *iter; | ||
| 4705 | struct ftrace_ops_hash old_hash_ops; | 4708 | struct ftrace_ops_hash old_hash_ops; |
| 4706 | struct ftrace_func_entry *entry; | 4709 | struct ftrace_func_entry *entry; |
| 4707 | struct ftrace_func_probe *probe; | ||
| 4708 | struct ftrace_glob func_g; | 4710 | struct ftrace_glob func_g; |
| 4709 | struct ftrace_hash **orig_hash; | 4711 | struct ftrace_hash **orig_hash; |
| 4710 | struct ftrace_hash *old_hash; | 4712 | struct ftrace_hash *old_hash; |
| @@ -4732,11 +4734,13 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr, · 21 unmodified lines | |||
| 4732 | 4734 | ||
| 4733 | mutex_lock(&ftrace_lock); | 4735 | mutex_lock(&ftrace_lock); |
| 4734 | /* Check if the probe_ops is already registered */ | 4736 | /* Check if the probe_ops is already registered */ |
| 4735 | list_for_each_entry(probe, &tr->func_probes, list) { | 4737 | list_for_each_entry(iter, &tr->func_probes, list) { |
| 4736 | if (probe->probe_ops == probe_ops) | 4738 | if (iter->probe_ops == probe_ops) { |
| 4739 | probe = iter; | ||
| 4737 | break; | 4740 | break; |
| 4741 | } | ||
| 4738 | } | 4742 | } |
| 4739 | if (&probe->list == &tr->func_probes) | 4743 | if (!probe) |
| 4740 | goto err_unlock_ftrace; | 4744 | goto err_unlock_ftrace; |
| 4741 | 4745 | ||
| 4742 | ret = -EINVAL; | 4746 | ret = -EINVAL; |
| @@ -648,7 +648,7 @@ static struct trace_event_functions eprobe_funcs = { · 647 unmodified lines | |||
| 648 | static int disable_eprobe(struct trace_eprobe *ep, | 648 | static int disable_eprobe(struct trace_eprobe *ep, |
| 649 | struct trace_array *tr) | 649 | struct trace_array *tr) |
| 650 | { | 650 | { |
| 651 | struct event_trigger_data *trigger; | 651 | struct event_trigger_data *trigger = NULL, *iter; |
| 652 | struct trace_event_file *file; | 652 | struct trace_event_file *file; |
| 653 | struct eprobe_data *edata; | 653 | struct eprobe_data *edata; |
| 654 | 654 | ||
| @@ -656,14 +656,16 @@ static int disable_eprobe(struct trace_eprobe *ep, · 1 unmodified line | |||
| 656 | if (!file) | 656 | if (!file) |
| 657 | return -ENOENT; | 657 | return -ENOENT; |
| 658 | 658 | ||
| 659 | list_for_each_entry(trigger, &file->triggers, list) { | 659 | list_for_each_entry(iter, &file->triggers, list) { |
| 660 | if (!(trigger->flags & EVENT_TRIGGER_FL_PROBE)) | 660 | if (!(iter->flags & EVENT_TRIGGER_FL_PROBE)) |
| 661 | continue; | 661 | continue; |
| 662 | edata = trigger->private_data; | 662 | edata = iter->private_data; |
| 663 | if (edata->ep == ep) | 663 | if (edata->ep == ep) { |
| 664 | trigger = iter; | ||
| 664 | break; | 665 | break; |
| 666 | } | ||
| 665 | } | 667 | } |
| 666 | if (list_entry_is_head(trigger, &file->triggers, list)) | 668 | if (!trigger) |
| 667 | return -ENODEV; | 669 | return -ENODEV; |
| 668 | 670 | ||
| 669 | list_del_rcu(&trigger->list); | 671 | list_del_rcu(&trigger->list); |
| @@ -2279,8 +2279,8 @@ static struct dentry * · 2278 unmodified lines | |||
| 2279 | event_subsystem_dir(struct trace_array *tr, const char *name, | 2279 | event_subsystem_dir(struct trace_array *tr, const char *name, |
| 2280 | struct trace_event_file *file, struct dentry *parent) | 2280 | struct trace_event_file *file, struct dentry *parent) |
| 2281 | { | 2281 | { |
| 2282 | struct event_subsystem *system, *iter; | ||
| 2282 | struct trace_subsystem_dir *dir; | 2283 | struct trace_subsystem_dir *dir; |
| 2283 | struct event_subsystem *system; | ||
| 2284 | struct dentry *entry; | 2284 | struct dentry *entry; |
| 2285 | 2285 | ||
| 2286 | /* First see if we did not already create this dir */ | 2286 | /* First see if we did not already create this dir */ |
| @@ -2294,13 +2294,13 @@ event_subsystem_dir(struct trace_array *tr, const char *name, · 7 unmodified lines | |||
| 2294 | } | 2294 | } |
| 2295 | 2295 | ||
| 2296 | /* Now see if the system itself exists. */ | 2296 | /* Now see if the system itself exists. */ |
| 2297 | list_for_each_entry(system, &event_subsystems, list) { | 2297 | system = NULL; |
| 2298 | if (strcmp(system->name, name) == 0) | 2298 | list_for_each_entry(iter, &event_subsystems, list) { |
| 2299 | if (strcmp(iter->name, name) == 0) { | ||
| 2300 | system = iter; | ||
| 2299 | break; | 2301 | break; |
| 2302 | } | ||
| 2300 | } | 2303 | } |
| 2301 | /* Reset system variable when not found */ | ||
| 2302 | if (&system->list == &event_subsystems) | ||
| 2303 | system = NULL; | ||
| 2304 | 2304 | ||
| 2305 | dir = kmalloc(sizeof(*dir), GFP_KERNEL); | 2305 | dir = kmalloc(sizeof(*dir), GFP_KERNEL); |
| 2306 | if (!dir) | 2306 | if (!dir) |