On Mon, Nov 05, 2018 at 04:58:14PM -0200, Julio Faracco wrote: > The array "mount" inside lxc_container is not being checked before for > loop. Clang syntax scan is complaining about this segmentation fault.
Which in this specific case is a false positive since nmounts pretty much protects us from that happening. Usually I'm not very keen on making changes to silence false positives from analyzers like coverity, however, in this case we can actually make things better I believe. > > Signed-off-by: Julio Faracco <[email protected]> > --- > src/lxc/lxc_container.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c > index 918194dacd..6b7bcd8eb6 100644 > --- a/src/lxc/lxc_container.c > +++ b/src/lxc/lxc_container.c > @@ -871,7 +871,7 @@ static int lxcContainerSetReadOnly(void) How about we revert the logic HERE (in the missing context) and instead do: if (!mounts) { ret = 0; goto cleanup; } > qsort(mounts, nmounts, sizeof(mounts[0]), > virStringSortRevCompare); > > - for (i = 0; i < nmounts; i++) { > + for (i = 0; i < nmounts && mounts; i++) { > VIR_DEBUG("Bind readonly %s", mounts[i]); > if (mount(mounts[i], mounts[i], "none", > MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) { > virReportSystemError(errno, > @@ -883,7 +883,7 @@ static int lxcContainerSetReadOnly(void) > > ret = 0; > cleanup: > - for (i = 0; i < nmounts; i++) > + for (i = 0; i < nmounts && mounts; i++) and then ^here replace this hunk with virStringListFreeCount which already checks @src for NULL, so that should suffice from Clang's perspective, right? Erik > VIR_FREE(mounts[i]); > VIR_FREE(mounts); > endmntent(procmnt); > -- > 2.17.1 > > -- > libvir-list mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
