All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libsepol: avoid passing NULL pointer to memcpy
@ 2021-10-21 14:08 Christian Göttsche
  2021-10-28 18:09 ` James Carter
  2021-11-05 15:29 ` [PATCH v2] " Christian Göttsche
  0 siblings, 2 replies; 5+ messages in thread
From: Christian Göttsche @ 2021-10-21 14:08 UTC (permalink / raw)
  To: selinux

memcpy(3) might be annotated with the function attribute nonnull and
UBSan then complains:

    module.c:296:3: runtime error: null pointer passed as argument 2, which is declared to never be null
        #0 0x7f2468efa5b3 in link_netfilter_contexts ./libsepol/src/module.c:296
        #1 0x7f2468efa5b3 in sepol_link_packages ./libsepol/src/module.c:337
        #2 0x562331e9e123 in main ./semodule-utils/semodule_link/semodule_link.c:145
        #3 0x7f2467e247ec in __libc_start_main ../csu/libc-start.c:332
        #4 0x562331e9d2a9 in _start (./destdir/usr/bin/semodule_link+0x32a9)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libsepol/src/module.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libsepol/src/module.c b/libsepol/src/module.c
index 02a5de2c..ed777f8e 100644
--- a/libsepol/src/module.c
+++ b/libsepol/src/module.c
@@ -293,9 +293,11 @@ static int link_netfilter_contexts(sepol_module_package_t * base,
 	}
 	base->netfilter_contexts = base_context;
 	for (i = 0; i < num_modules; i++) {
-		memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
-		       modules[i]->netfilter_contexts,
-		       modules[i]->netfilter_contexts_len);
+		if (modules[i]->netfilter_contexts_len > 0) {
+			memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
+			       modules[i]->netfilter_contexts,
+			       modules[i]->netfilter_contexts_len);
+		}
 		base->netfilter_contexts_len +=
 		    modules[i]->netfilter_contexts_len;
 	}
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] libsepol: avoid passing NULL pointer to memcpy
  2021-10-21 14:08 [PATCH] libsepol: avoid passing NULL pointer to memcpy Christian Göttsche
@ 2021-10-28 18:09 ` James Carter
  2021-11-05 15:29 ` [PATCH v2] " Christian Göttsche
  1 sibling, 0 replies; 5+ messages in thread
From: James Carter @ 2021-10-28 18:09 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: SElinux list

On Thu, Oct 21, 2021 at 10:09 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> memcpy(3) might be annotated with the function attribute nonnull and
> UBSan then complains:
>
>     module.c:296:3: runtime error: null pointer passed as argument 2, which is declared to never be null
>         #0 0x7f2468efa5b3 in link_netfilter_contexts ./libsepol/src/module.c:296
>         #1 0x7f2468efa5b3 in sepol_link_packages ./libsepol/src/module.c:337
>         #2 0x562331e9e123 in main ./semodule-utils/semodule_link/semodule_link.c:145
>         #3 0x7f2467e247ec in __libc_start_main ../csu/libc-start.c:332
>         #4 0x562331e9d2a9 in _start (./destdir/usr/bin/semodule_link+0x32a9)
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  libsepol/src/module.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/libsepol/src/module.c b/libsepol/src/module.c
> index 02a5de2c..ed777f8e 100644
> --- a/libsepol/src/module.c
> +++ b/libsepol/src/module.c
> @@ -293,9 +293,11 @@ static int link_netfilter_contexts(sepol_module_package_t * base,
>         }
>         base->netfilter_contexts = base_context;
>         for (i = 0; i < num_modules; i++) {
> -               memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
> -                      modules[i]->netfilter_contexts,
> -                      modules[i]->netfilter_contexts_len);
> +               if (modules[i]->netfilter_contexts_len > 0) {
> +                       memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
> +                              modules[i]->netfilter_contexts,
> +                              modules[i]->netfilter_contexts_len);
> +               }
>                 base->netfilter_contexts_len +=
>                     modules[i]->netfilter_contexts_len;

This line should be in the if statement as well. There is no point in
adding a 0, since the check for > 0 is already being made.
Jim


>         }
> --
> 2.33.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] libsepol: avoid passing NULL pointer to memcpy
  2021-10-21 14:08 [PATCH] libsepol: avoid passing NULL pointer to memcpy Christian Göttsche
  2021-10-28 18:09 ` James Carter
@ 2021-11-05 15:29 ` Christian Göttsche
  2021-11-08 21:40   ` Nicolas Iooss
  1 sibling, 1 reply; 5+ messages in thread
From: Christian Göttsche @ 2021-11-05 15:29 UTC (permalink / raw)
  To: selinux

memcpy(3) might be annotated with the function attribute nonnull and
UBSan then complains:

    module.c:296:3: runtime error: null pointer passed as argument 2, which is declared to never be null
        #0 0x7f2468efa5b3 in link_netfilter_contexts ./libsepol/src/module.c:296
        #1 0x7f2468efa5b3 in sepol_link_packages ./libsepol/src/module.c:337
        #2 0x562331e9e123 in main ./semodule-utils/semodule_link/semodule_link.c:145
        #3 0x7f2467e247ec in __libc_start_main ../csu/libc-start.c:332
        #4 0x562331e9d2a9 in _start (./destdir/usr/bin/semodule_link+0x32a9)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

---
v2:
  include length addition into if block

---
 libsepol/src/module.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/libsepol/src/module.c b/libsepol/src/module.c
index 02a5de2c..b718751e 100644
--- a/libsepol/src/module.c
+++ b/libsepol/src/module.c
@@ -293,11 +293,14 @@ static int link_netfilter_contexts(sepol_module_package_t * base,
 	}
 	base->netfilter_contexts = base_context;
 	for (i = 0; i < num_modules; i++) {
-		memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
-		       modules[i]->netfilter_contexts,
-		       modules[i]->netfilter_contexts_len);
-		base->netfilter_contexts_len +=
-		    modules[i]->netfilter_contexts_len;
+		if (modules[i]->netfilter_contexts_len > 0) {
+			memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
+			       modules[i]->netfilter_contexts,
+			       modules[i]->netfilter_contexts_len);
+			base->netfilter_contexts_len +=
+			    modules[i]->netfilter_contexts_len;
+		}
+
 	}
 	return 0;
 }
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] libsepol: avoid passing NULL pointer to memcpy
  2021-11-05 15:29 ` [PATCH v2] " Christian Göttsche
@ 2021-11-08 21:40   ` Nicolas Iooss
  2021-11-11 22:02     ` Nicolas Iooss
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Iooss @ 2021-11-08 21:40 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: SElinux list

On Fri, Nov 5, 2021 at 4:29 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> memcpy(3) might be annotated with the function attribute nonnull and
> UBSan then complains:
>
>     module.c:296:3: runtime error: null pointer passed as argument 2, which is declared to never be null
>         #0 0x7f2468efa5b3 in link_netfilter_contexts ./libsepol/src/module.c:296
>         #1 0x7f2468efa5b3 in sepol_link_packages ./libsepol/src/module.c:337
>         #2 0x562331e9e123 in main ./semodule-utils/semodule_link/semodule_link.c:145
>         #3 0x7f2467e247ec in __libc_start_main ../csu/libc-start.c:332
>         #4 0x562331e9d2a9 in _start (./destdir/usr/bin/semodule_link+0x32a9)
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Thanks!
Nicolas

> ---
> v2:
>   include length addition into if block
>
> ---
>  libsepol/src/module.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/libsepol/src/module.c b/libsepol/src/module.c
> index 02a5de2c..b718751e 100644
> --- a/libsepol/src/module.c
> +++ b/libsepol/src/module.c
> @@ -293,11 +293,14 @@ static int link_netfilter_contexts(sepol_module_package_t * base,
>         }
>         base->netfilter_contexts = base_context;
>         for (i = 0; i < num_modules; i++) {
> -               memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
> -                      modules[i]->netfilter_contexts,
> -                      modules[i]->netfilter_contexts_len);
> -               base->netfilter_contexts_len +=
> -                   modules[i]->netfilter_contexts_len;
> +               if (modules[i]->netfilter_contexts_len > 0) {
> +                       memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
> +                              modules[i]->netfilter_contexts,
> +                              modules[i]->netfilter_contexts_len);
> +                       base->netfilter_contexts_len +=
> +                           modules[i]->netfilter_contexts_len;
> +               }
> +
>         }
>         return 0;
>  }
> --
> 2.33.1
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] libsepol: avoid passing NULL pointer to memcpy
  2021-11-08 21:40   ` Nicolas Iooss
@ 2021-11-11 22:02     ` Nicolas Iooss
  0 siblings, 0 replies; 5+ messages in thread
From: Nicolas Iooss @ 2021-11-11 22:02 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: SElinux list

On Mon, Nov 8, 2021 at 10:40 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Fri, Nov 5, 2021 at 4:29 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > memcpy(3) might be annotated with the function attribute nonnull and
> > UBSan then complains:
> >
> >     module.c:296:3: runtime error: null pointer passed as argument 2, which is declared to never be null
> >         #0 0x7f2468efa5b3 in link_netfilter_contexts ./libsepol/src/module.c:296
> >         #1 0x7f2468efa5b3 in sepol_link_packages ./libsepol/src/module.c:337
> >         #2 0x562331e9e123 in main ./semodule-utils/semodule_link/semodule_link.c:145
> >         #3 0x7f2467e247ec in __libc_start_main ../csu/libc-start.c:332
> >         #4 0x562331e9d2a9 in _start (./destdir/usr/bin/semodule_link+0x32a9)
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Thanks!
> Nicolas

This patch is now applied.

Thanks!
Nicolas

> > ---
> > v2:
> >   include length addition into if block
> >
> > ---
> >  libsepol/src/module.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/libsepol/src/module.c b/libsepol/src/module.c
> > index 02a5de2c..b718751e 100644
> > --- a/libsepol/src/module.c
> > +++ b/libsepol/src/module.c
> > @@ -293,11 +293,14 @@ static int link_netfilter_contexts(sepol_module_package_t * base,
> >         }
> >         base->netfilter_contexts = base_context;
> >         for (i = 0; i < num_modules; i++) {
> > -               memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
> > -                      modules[i]->netfilter_contexts,
> > -                      modules[i]->netfilter_contexts_len);
> > -               base->netfilter_contexts_len +=
> > -                   modules[i]->netfilter_contexts_len;
> > +               if (modules[i]->netfilter_contexts_len > 0) {
> > +                       memcpy(base->netfilter_contexts + base->netfilter_contexts_len,
> > +                              modules[i]->netfilter_contexts,
> > +                              modules[i]->netfilter_contexts_len);
> > +                       base->netfilter_contexts_len +=
> > +                           modules[i]->netfilter_contexts_len;
> > +               }
> > +
> >         }
> >         return 0;
> >  }
> > --
> > 2.33.1
> >


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-11 22:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 14:08 [PATCH] libsepol: avoid passing NULL pointer to memcpy Christian Göttsche
2021-10-28 18:09 ` James Carter
2021-11-05 15:29 ` [PATCH v2] " Christian Göttsche
2021-11-08 21:40   ` Nicolas Iooss
2021-11-11 22:02     ` Nicolas Iooss

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.