diff mbox series

[09/14] tracing/string: Create and use __free(argv_free) in trace_dynevent.c

Message ID 20241219201346.029319524@goodmis.org (mailing list archive)
State New
Headers show
Series None | expand

Commit Message

Steven Rostedt Dec. 19, 2024, 8:12 p.m. UTC 
From: Steven Rostedt <rostedt@goodmis.org>

The function dyn_event_release() uses argv_split() which must be freed via
argv_free(). It contains several error paths that do a goto out to call
argv_free() for cleanup. This makes the code complex and error prone.

Create a new __free() directive __free(argv_free) that will call
argv_free() for data allocated with argv_split(), and use it in the
dyn_event_release() function.

Cc: Kees Cook <kees@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/linux/string.h        |  3 +++
 kernel/trace/trace_dynevent.c | 23 +++++++----------------
 2 files changed, 10 insertions(+), 16 deletions(-)

Comments

Steven Rostedt Dec. 20, 2024, 1:31 p.m. UTC | #1 
On Fri, 20 Dec 2024 08:56:44 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> > diff --git a/include/linux/string.h b/include/linux/string.h
> > index 493ac4862c77..c995f973a59a 100644
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -10,6 +10,7 @@
> >  #include <linux/err.h>         /* for ERR_PTR() */
> >  #include <linux/errno.h>       /* for E2BIG */
> >  #include <linux/overflow.h>    /* for check_mul_overflow() */
> > +#include <linux/cleanup.h>     /* for DEFINE_FREE() */  
> 
> 
> 
> Can you keep it ordered?

Not sure what order you want to keep?

> 
> 
> >  #include <linux/stdarg.h>
> >  #include <uapi/linux/string.h>
> >


#include <linux/args.h>
#include <linux/array_size.h>
#include <linux/compiler.h>	/* for inline */
#include <linux/types.h>	/* for size_t */
#include <linux/stddef.h>	/* for NULL */
#include <linux/err.h>		/* for ERR_PTR() */
#include <linux/errno.h>	/* for E2BIG */
#include <linux/overflow.h>	/* for check_mul_overflow() */
#include <linux/cleanup.h>	/* for DEFINE_FREE() */
#include <linux/stdarg.h>
#include <uapi/linux/string.h>

It's not alphabetical, nor in x-mas tree order. I was never good at these
"find the pattern" puzzles ;-)

-- Steve
Steven Rostedt Dec. 20, 2024, 3:28 p.m. UTC | #2 
On Fri, 20 Dec 2024 15:48:44 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> пʼятниця, 20 грудня 2024 р. Steven Rostedt <rostedt@goodmis.org> пише:
> 
> > On Fri, 20 Dec 2024 08:56:44 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >  
> > > > diff --git a/include/linux/string.h b/include/linux/string.h
> > > > index 493ac4862c77..c995f973a59a 100644
> > > > --- a/include/linux/string.h
> > > > +++ b/include/linux/string.h
> > > > @@ -10,6 +10,7 @@
> > > >  #include <linux/err.h>         /* for ERR_PTR() */
> > > >  #include <linux/errno.h>       /* for E2BIG */
> > > >  #include <linux/overflow.h>    /* for check_mul_overflow() */
> > > > +#include <linux/cleanup.h>     /* for DEFINE_FREE() */  
> > >
> > >
> > >
> > > Can you keep it ordered?  
> >
> > Not sure what order you want to keep?
> >  
> > >
> > >  
> > > >  #include <linux/stdarg.h>
> > > >  #include <uapi/linux/string.h>
> > > >  
> >
> >
> > #include <linux/args.h>
> > #include <linux/array_size.h>  
> 
> 
> 
> If you put it here it would make more like alphabetical order with only few
> exceptions

Sure, I'll send out a v2. Thanks for the review.

--Steve

> 
> #include <linux/compiler.h>     /* for inline */
> > #include <linux/types.h>        /* for size_t */
> > #include <linux/stddef.h>       /* for NULL */
> > #include <linux/err.h>          /* for ERR_PTR() */
> > #include <linux/errno.h>        /* for E2BIG */
> > #include <linux/overflow.h>     /* for check_mul_overflow() */
> > #include <linux/cleanup.h>      /* for DEFINE_FREE() */
> > #include <linux/stdarg.h>
> > #include <uapi/linux/string.h>
> >
> > It's not alphabetical, nor in x-mas tree order. I was never good at these
> > "find the pattern" puzzles ;-)
> >
> > -- Steve
> >
diff mbox series

Patch

diff --git a/include/linux/string.h b/include/linux/string.h
index 493ac4862c77..c995f973a59a 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -10,6 +10,7 @@ 
 #include <linux/err.h>		/* for ERR_PTR() */
 #include <linux/errno.h>	/* for E2BIG */
 #include <linux/overflow.h>	/* for check_mul_overflow() */
+#include <linux/cleanup.h>	/* for DEFINE_FREE() */
 #include <linux/stdarg.h>
 #include <uapi/linux/string.h>
 
@@ -312,6 +313,8 @@  extern void *kmemdup_array(const void *src, size_t count, size_t element_size, g
 extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
 extern void argv_free(char **argv);
 
+DEFINE_FREE(argv_free, char **, if (!IS_ERR_OR_NULL(_T)) argv_free(_T))
+
 /* lib/cmdline.c */
 extern int get_option(char **str, int *pint);
 extern char *get_options(const char *str, int nints, int *ints);
diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index 4376887e0d8a..a322e4f249a5 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -74,24 +74,19 @@  int dyn_event_release(const char *raw_command, struct dyn_event_operations *type
 	struct dyn_event *pos, *n;
 	char *system = NULL, *event, *p;
 	int argc, ret = -ENOENT;
-	char **argv;
+	char **argv __free(argv_free) = argv_split(GFP_KERNEL, raw_command, &argc);
 
-	argv = argv_split(GFP_KERNEL, raw_command, &argc);
 	if (!argv)
 		return -ENOMEM;
 
 	if (argv[0][0] == '-') {
-		if (argv[0][1] != ':') {
-			ret = -EINVAL;
-			goto out;
-		}
+		if (argv[0][1] != ':')
+			return -EINVAL;
 		event = &argv[0][2];
 	} else {
 		event = strchr(argv[0], ':');
-		if (!event) {
-			ret = -EINVAL;
-			goto out;
-		}
+		if (!event)
+			return -EINVAL;
 		event++;
 	}
 
@@ -101,10 +96,8 @@  int dyn_event_release(const char *raw_command, struct dyn_event_operations *type
 		event = p + 1;
 		*p = '\0';
 	}
-	if (!system && event[0] == '\0') {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (!system && event[0] == '\0')
+		return -EINVAL;
 
 	mutex_lock(&event_mutex);
 	for_each_dyn_event_safe(pos, n) {
@@ -120,8 +113,6 @@  int dyn_event_release(const char *raw_command, struct dyn_event_operations *type
 	}
 	tracing_reset_all_online_cpus();
 	mutex_unlock(&event_mutex);
-out:
-	argv_free(argv);
 	return ret;
 }