linux-kernel - Re: [External] Re: [PATCH 1/2] ftrace: clear module from hash of all ftrace ops

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]

Message-ID: <20200728132630.678f94f7@oasis.local.home>
Date:   Tue, 28 Jul 2020 13:26:30 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Chengming Zhou <zhouchengming@...edance.com>
Cc:     mingo@...hat.com, linux-kernel@...r.kernel.org,
        songmuchun@...edance.com
Subject: Re: [External] Re: [PATCH 1/2] ftrace: clear module from hash of
 all ftrace ops

On Wed, 29 Jul 2020 00:59:33 +0800
Chengming Zhou <zhouchengming@...edance.com> wrote:


> > i.e.
> >
> >   # echo some_module_function > set_ftrace_filter
> >   # rmmod module_with_that_function
> >   # insmod module_with_same_address_of_function
> >   # echo function > current_tracer
> >
> > Now the tr->ops->hash would still have the function of the original
> > module.  
> 
> I thought all ftrace_ops has non empty func_hash are on the ftrace 
> global list...

Nope, the two are disjoint.

> 
> Well, so I just leave this function unmodified.
> 
> Just call that new function register_ftrace_ops_hash() from 
> ftrace_release_mod.

I would say to have anything that uses one of the
ftrace_set_filter/notrace* functions, to also register itself for
module removal.

	register_ftrace_mod_removal(struct ftrace_ops *ops);

and then also have a unregister_ftrace_mod_removal() as there needs to
be a way to remove it from the list before the ops gets freed.

Then these functions would add the ops to a list, and this list is
traversed to remove modules. The trace_arrays can register their ops
too, so you can update that function.

-- Steve


> 
> Thanks!
> 
> >
> > Either have all owners of ftrace_ops handle this case, or add a helper
> > function to handle it for them. But using ftarce_ops_list is the wrong
> > place to do it.
> >
> > -- Steve
> >
> >  
> >> +	mutex_lock(&ftrace_lock);
> >> +
> >> +	do_for_each_ftrace_op(op, ftrace_ops_list) {
> >> +		if (!op->func_hash)
> >>   			continue;
> >> -		mutex_lock(&tr->ops->func_hash->regex_lock);
> >> -		clear_mod_from_hash(pg, tr->ops->func_hash->filter_hash);
> >> -		clear_mod_from_hash(pg, tr->ops->func_hash->notrace_hash);
> >> -		mutex_unlock(&tr->ops->func_hash->regex_lock);
> >> -	}
> >> -	mutex_unlock(&trace_types_lock);
> >> +		mutex_lock(&op->func_hash->regex_lock);
> >> +		clear_mod_from_hash(pg, op->func_hash->filter_hash);
> >> +		clear_mod_from_hash(pg, op->func_hash->notrace_hash);
> >> +		mutex_unlock(&op->func_hash->regex_lock);
> >> +	} while_for_each_ftrace_op(op);
> >> +
> >> +	mutex_unlock(&ftrace_lock);
> >>   }
> >>   
> >>   static void ftrace_free_mod_map(struct rcu_head *rcu)