On Wed, Sep 06, 2023 at 18:34:42 +0300, Dmitry Frolov wrote: > Reviewing the sources, I found, that > in function lxcContainerMountAllFS() pointers > vmDef->fss[i]->src and vmDef->fss[i]->src->path > are checked for NULL after dereferencing in > VIR_DEBUG() macro. > > Fixes: 57487085dc ("lxc: don't try to reference NULL when mounting filesystems") > --- > src/lxc/lxc_container.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c > index 21220661f7..58a6695458 100644 > --- a/src/lxc/lxc_container.c > +++ b/src/lxc/lxc_container.c > @@ -1467,13 +1467,15 @@ static int lxcContainerMountAllFS(virDomainDef *vmDef, > if (STREQ(vmDef->fss[i]->dst, "/")) > continue; > > + if (!(vmDef->fss[i]->src && vmDef->fss[i]->src->path)) > + return -1; This would return -1 without reporting any error. And you are also changing the logic here without explaining it. If vmDef->fss[i]->src was NULL or vmDef->fss[i]->src->path was NULL this function would call lxcContainerUnmountSubtree on vmDef->fss[i]->dst before, but now it just returns -1. Well except that it would crash first :-) In any case, this part is wrong, see below... > + > VIR_DEBUG("Mounting '%s' -> '%s'", vmDef->fss[i]->src->path, vmDef->fss[i]->dst); > > if (lxcContainerResolveSymlinks(vmDef->fss[i], false) < 0) > return -1; > > - if (!(vmDef->fss[i]->src && vmDef->fss[i]->src->path && > - STRPREFIX(vmDef->fss[i]->src->path, vmDef->fss[i]->dst)) && > + if (!STRPREFIX(vmDef->fss[i]->src->path, vmDef->fss[i]->dst) && > lxcContainerUnmountSubtree(vmDef->fss[i]->dst, false) < 0) > return -1; I checked our parsing code and vmDef->fss[i]->src can never be NULL as it is always allocated in virDomainFSDefNew so checking it is redundant. On the other hand vmDef->fss[i]->src->path can be NULL, but it is not en error, e.g., VIR_DOMAIN_FS_TYPE_RAM would contain path == NULL. So the fix should rather remove the check for vmDef->fss[i]->src and use NULLSTR(vmDef->fss[i]->src->path) in VIR_DEBUG. Jirka