All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] lib subcmd: Avoid segv/use-after-free when commands aren't excluded
@ 2023-07-07 23:09 Ian Rogers
  2023-07-11 17:36 ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2023-07-07 23:09 UTC (permalink / raw)
  To: Ian Rogers, Namhyung Kim, Arnaldo Carvalho de Melo, Chenyuan Mi,
	linux-kernel

The array shortening may perform unnecessary array copies. Before
commit 657a3efee43a ("lib subcmd: Avoid memory leak in exclude_cmds")
this was benign, but afterwards this could lead to a segv.

Fixes: 657a3efee43a ("lib subcmd: Avoid memory leak in exclude_cmds")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/subcmd/help.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
index 67a8d6b740ea..adfbae27dc36 100644
--- a/tools/lib/subcmd/help.c
+++ b/tools/lib/subcmd/help.c
@@ -68,8 +68,13 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
 	while (ci < cmds->cnt && ei < excludes->cnt) {
 		cmp = strcmp(cmds->names[ci]->name, excludes->names[ei]->name);
 		if (cmp < 0) {
-			zfree(&cmds->names[cj]);
-			cmds->names[cj++] = cmds->names[ci++];
+			if (ci == cj) {
+				ci++;
+				cj++;
+			} else {
+				zfree(&cmds->names[cj]);
+				cmds->names[cj++] = cmds->names[ci++];
+			}
 		} else if (cmp == 0) {
 			ci++;
 			ei++;
@@ -77,10 +82,11 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
 			ei++;
 		}
 	}
-
-	while (ci < cmds->cnt) {
-		zfree(&cmds->names[cj]);
-		cmds->names[cj++] = cmds->names[ci++];
+	if (ci != cj) {
+		while (ci < cmds->cnt) {
+			zfree(&cmds->names[cj]);
+			cmds->names[cj++] = cmds->names[ci++];
+		}
 	}
 	for (ci = cj; ci < cmds->cnt; ci++)
 		zfree(&cmds->names[ci]);
-- 
2.41.0.390.g38632f3daf-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] lib subcmd: Avoid segv/use-after-free when commands aren't excluded
  2023-07-07 23:09 [PATCH v2] lib subcmd: Avoid segv/use-after-free when commands aren't excluded Ian Rogers
@ 2023-07-11 17:36 ` Namhyung Kim
  2023-07-11 19:46   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2023-07-11 17:36 UTC (permalink / raw)
  To: Ian Rogers; +Cc: Arnaldo Carvalho de Melo, Chenyuan Mi, linux-kernel

On Fri, Jul 7, 2023 at 4:09 PM Ian Rogers <irogers@google.com> wrote:
>
> The array shortening may perform unnecessary array copies. Before
> commit 657a3efee43a ("lib subcmd: Avoid memory leak in exclude_cmds")
> this was benign, but afterwards this could lead to a segv.
>
> Fixes: 657a3efee43a ("lib subcmd: Avoid memory leak in exclude_cmds")
> Signed-off-by: Ian Rogers <irogers@google.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/lib/subcmd/help.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> index 67a8d6b740ea..adfbae27dc36 100644
> --- a/tools/lib/subcmd/help.c
> +++ b/tools/lib/subcmd/help.c
> @@ -68,8 +68,13 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
>         while (ci < cmds->cnt && ei < excludes->cnt) {
>                 cmp = strcmp(cmds->names[ci]->name, excludes->names[ei]->name);
>                 if (cmp < 0) {
> -                       zfree(&cmds->names[cj]);
> -                       cmds->names[cj++] = cmds->names[ci++];
> +                       if (ci == cj) {
> +                               ci++;
> +                               cj++;
> +                       } else {
> +                               zfree(&cmds->names[cj]);
> +                               cmds->names[cj++] = cmds->names[ci++];
> +                       }
>                 } else if (cmp == 0) {
>                         ci++;
>                         ei++;
> @@ -77,10 +82,11 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
>                         ei++;
>                 }
>         }
> -
> -       while (ci < cmds->cnt) {
> -               zfree(&cmds->names[cj]);
> -               cmds->names[cj++] = cmds->names[ci++];
> +       if (ci != cj) {
> +               while (ci < cmds->cnt) {
> +                       zfree(&cmds->names[cj]);
> +                       cmds->names[cj++] = cmds->names[ci++];
> +               }
>         }
>         for (ci = cj; ci < cmds->cnt; ci++)
>                 zfree(&cmds->names[ci]);
> --
> 2.41.0.390.g38632f3daf-goog
>

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] lib subcmd: Avoid segv/use-after-free when commands aren't excluded
  2023-07-11 17:36 ` Namhyung Kim
@ 2023-07-11 19:46   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-07-11 19:46 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ian Rogers, Arnaldo Carvalho de Melo, Chenyuan Mi, linux-kernel

Em Tue, Jul 11, 2023 at 10:36:59AM -0700, Namhyung Kim escreveu:
> On Fri, Jul 7, 2023 at 4:09 PM Ian Rogers <irogers@google.com> wrote:
> >
> > The array shortening may perform unnecessary array copies. Before
> > commit 657a3efee43a ("lib subcmd: Avoid memory leak in exclude_cmds")
> > this was benign, but afterwards this could lead to a segv.
> >
> > Fixes: 657a3efee43a ("lib subcmd: Avoid memory leak in exclude_cmds")
> > Signed-off-by: Ian Rogers <irogers@google.com>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied to perf-tools.

- Arnaldo

 
> Thanks,
> Namhyung
> 
> 
> > ---
> >  tools/lib/subcmd/help.c | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> > index 67a8d6b740ea..adfbae27dc36 100644
> > --- a/tools/lib/subcmd/help.c
> > +++ b/tools/lib/subcmd/help.c
> > @@ -68,8 +68,13 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
> >         while (ci < cmds->cnt && ei < excludes->cnt) {
> >                 cmp = strcmp(cmds->names[ci]->name, excludes->names[ei]->name);
> >                 if (cmp < 0) {
> > -                       zfree(&cmds->names[cj]);
> > -                       cmds->names[cj++] = cmds->names[ci++];
> > +                       if (ci == cj) {
> > +                               ci++;
> > +                               cj++;
> > +                       } else {
> > +                               zfree(&cmds->names[cj]);
> > +                               cmds->names[cj++] = cmds->names[ci++];
> > +                       }
> >                 } else if (cmp == 0) {
> >                         ci++;
> >                         ei++;
> > @@ -77,10 +82,11 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
> >                         ei++;
> >                 }
> >         }
> > -
> > -       while (ci < cmds->cnt) {
> > -               zfree(&cmds->names[cj]);
> > -               cmds->names[cj++] = cmds->names[ci++];
> > +       if (ci != cj) {
> > +               while (ci < cmds->cnt) {
> > +                       zfree(&cmds->names[cj]);
> > +                       cmds->names[cj++] = cmds->names[ci++];
> > +               }
> >         }
> >         for (ci = cj; ci < cmds->cnt; ci++)
> >                 zfree(&cmds->names[ci]);
> > --
> > 2.41.0.390.g38632f3daf-goog
> >

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-11 19:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-07 23:09 [PATCH v2] lib subcmd: Avoid segv/use-after-free when commands aren't excluded Ian Rogers
2023-07-11 17:36 ` Namhyung Kim
2023-07-11 19:46   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.