Re: [RFC PATCH 1/9] fprobe: Pass entry_data to handlers

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Tue, 17 Jan 2023 20:43:09 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed,  9 Nov 2022 00:49:23 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> > index e8143e368074..fa25d09c9d57 100644
> > --- a/kernel/trace/fprobe.c
> > +++ b/kernel/trace/fprobe.c
> > @@ -17,14 +17,16 @@
> >  struct fprobe_rethook_node {
> >  	struct rethook_node node;
> >  	unsigned long entry_ip;
> > +	char data[];
> >  };
> >  
> >  static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
> >  			   struct ftrace_ops *ops, struct ftrace_regs *fregs)
> >  {
> >  	struct fprobe_rethook_node *fpr;
> > -	struct rethook_node *rh;
> > +	struct rethook_node *rh = NULL;
> >  	struct fprobe *fp;
> > +	void *entry_data = NULL;
> >  	int bit;
> >  
> >  	fp = container_of(ops, struct fprobe, ops);
> > @@ -37,9 +39,6 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
> >  		return;
> >  	}
> >  
> > -	if (fp->entry_handler)
> > -		fp->entry_handler(fp, ip, ftrace_get_regs(fregs));
> > -
> >  	if (fp->exit_handler) {
> >  		rh = rethook_try_get(fp->rethook);
> >  		if (!rh) {
> > @@ -48,9 +47,16 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
> >  		}
> >  		fpr = container_of(rh, struct fprobe_rethook_node, node);
> >  		fpr->entry_ip = ip;
> > -		rethook_hook(rh, ftrace_get_regs(fregs), true);
> > +		if (fp->entry_data_size)
> > +			entry_data = fpr->data;
> >  	}
> >  
> > +	if (fp->entry_handler)
> > +		fp->entry_handler(fp, ip, ftrace_get_regs(fregs), entry_data);
> > +
> > +	if (rh)
> > +		rethook_hook(rh, ftrace_get_regs(fregs), true);
> > +
> >  out:
> >  	ftrace_test_recursion_unlock(bit);
> >  }
> > @@ -81,7 +87,8 @@ static void fprobe_exit_handler(struct rethook_node *rh, void *data,
> >  
> >  	fpr = container_of(rh, struct fprobe_rethook_node, node);
> >  
> > -	fp->exit_handler(fp, fpr->entry_ip, regs);
> > +	fp->exit_handler(fp, fpr->entry_ip, regs,
> > +			 fp->entry_data_size ? (void *)fpr->data : NULL);
> >  }
> >  NOKPROBE_SYMBOL(fprobe_exit_handler);
> >  
> > @@ -146,7 +153,7 @@ static int fprobe_init_rethook(struct fprobe *fp, int num)
> >  	for (i = 0; i < size; i++) {
> >  		struct fprobe_rethook_node *node;
> >  
> > -		node = kzalloc(sizeof(*node), GFP_KERNEL);
> > +		node = kzalloc(sizeof(*node) + fp->entry_data_size, GFP_KERNEL);
> 
> 		node = kzalloc(struct_size(node, data, fp->entry_data_size), GFP_KERNEL);
> 
> Should use struct_size() for any tail structure allocations.

OK, I'll use that.

Thank you!

> 
> -- Steve
> 
> 
> >  		if (!node) {
> >  			rethook_free(fp->rethook);
> >  			fp->rethook = NULL;
> > diff --git a/lib/test_fprobe.c b/lib/test_fprobe.c
> > index e0381b3ec410..34fa5a5bbda1 100644


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>