| Message ID | 20211105152937.36412-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] libsepol: avoid passing NULL pointer to memcpy | expand |
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 >
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 > >
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; }
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(-)