diff mbox series

[v2,1/3] libsepol: do not pass NULL to memcpy

Message ID 20211019151123.10335-1-cgzones@googlemail.com (mailing list archive)
State Accepted
Headers show
Series [v2,1/3] libsepol: do not pass NULL to memcpy | expand

Commit Message

Christian Göttsche Oct. 19, 2021, 3:11 p.m. UTC
For the first iteration `mod->perm_map[sclassi]` is NULL, thus do not
use it as source of a memcpy(3), even with a size of 0.  memcpy(3) might
be annotated with the function attribute nonnull and UBSan then
complains:

    link.c:193:3: runtime error: null pointer passed as argument 2, which is declared to never be null

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

---
v2:
   drop realloc rewrite, just check for 0 size
---
 libsepol/src/link.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Nicolas Iooss Nov. 8, 2021, 9:38 p.m. UTC | #1
On Tue, Oct 19, 2021 at 5:13 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> For the first iteration `mod->perm_map[sclassi]` is NULL, thus do not
> use it as source of a memcpy(3), even with a size of 0.  memcpy(3) might
> be annotated with the function attribute nonnull and UBSan then
> complains:
>
>     link.c:193:3: runtime error: null pointer passed as argument 2, which is declared to never be null
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

For these 3 patches:

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

Sorry for the delay, I have been busy with other topics.

Thanks!
Nicolas

> ---
> v2:
>    drop realloc rewrite, just check for 0 size
> ---
>  libsepol/src/link.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libsepol/src/link.c b/libsepol/src/link.c
> index 7512a4d9..b14240d5 100644
> --- a/libsepol/src/link.c
> +++ b/libsepol/src/link.c
> @@ -190,8 +190,9 @@ static int permission_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
>                         ERR(state->handle, "Out of memory!");
>                         return -1;
>                 }
> -               memcpy(newmap, mod->perm_map[sclassi],
> -                      mod->perm_map_len[sclassi] * sizeof(*newmap));
> +               if (mod->perm_map_len[sclassi] > 0) {
> +                       memcpy(newmap, mod->perm_map[sclassi], mod->perm_map_len[sclassi] * sizeof(*newmap));
> +               }
>                 free(mod->perm_map[sclassi]);
>                 mod->perm_map[sclassi] = newmap;
>                 mod->perm_map_len[sclassi] = perm->s.value;
> --
> 2.33.0
>
Nicolas Iooss Nov. 11, 2021, 10:01 p.m. UTC | #2
On Mon, Nov 8, 2021 at 10:38 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Tue, Oct 19, 2021 at 5:13 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > For the first iteration `mod->perm_map[sclassi]` is NULL, thus do not
> > use it as source of a memcpy(3), even with a size of 0.  memcpy(3) might
> > be annotated with the function attribute nonnull and UBSan then
> > complains:
> >
> >     link.c:193:3: runtime error: null pointer passed as argument 2, which is declared to never be null
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> For these 3 patches:
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> Sorry for the delay, I have been busy with other topics.
>
> Thanks!
> Nicolas

I applied these 3 patches.

Thanks!
Nicolas

> > ---
> > v2:
> >    drop realloc rewrite, just check for 0 size
> > ---
> >  libsepol/src/link.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libsepol/src/link.c b/libsepol/src/link.c
> > index 7512a4d9..b14240d5 100644
> > --- a/libsepol/src/link.c
> > +++ b/libsepol/src/link.c
> > @@ -190,8 +190,9 @@ static int permission_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
> >                         ERR(state->handle, "Out of memory!");
> >                         return -1;
> >                 }
> > -               memcpy(newmap, mod->perm_map[sclassi],
> > -                      mod->perm_map_len[sclassi] * sizeof(*newmap));
> > +               if (mod->perm_map_len[sclassi] > 0) {
> > +                       memcpy(newmap, mod->perm_map[sclassi], mod->perm_map_len[sclassi] * sizeof(*newmap));
> > +               }
> >                 free(mod->perm_map[sclassi]);
> >                 mod->perm_map[sclassi] = newmap;
> >                 mod->perm_map_len[sclassi] = perm->s.value;
> > --
> > 2.33.0
> >
diff mbox series

Patch

diff --git a/libsepol/src/link.c b/libsepol/src/link.c
index 7512a4d9..b14240d5 100644
--- a/libsepol/src/link.c
+++ b/libsepol/src/link.c
@@ -190,8 +190,9 @@  static int permission_copy_callback(hashtab_key_t key, hashtab_datum_t datum,
 			ERR(state->handle, "Out of memory!");
 			return -1;
 		}
-		memcpy(newmap, mod->perm_map[sclassi],
-		       mod->perm_map_len[sclassi] * sizeof(*newmap));
+		if (mod->perm_map_len[sclassi] > 0) {
+			memcpy(newmap, mod->perm_map[sclassi], mod->perm_map_len[sclassi] * sizeof(*newmap));
+		}
 		free(mod->perm_map[sclassi]);
 		mod->perm_map[sclassi] = newmap;
 		mod->perm_map_len[sclassi] = perm->s.value;