[PATCH] lxc: fix lxcContainerMountAllFS() DEREF_BEFORE_CHECK

[Martin Kletzander]

On Thu, Sep 07, 2023 at 09:25:14AM +0200, Peter Krempa wrote:
>On Thu, Sep 07, 2023 at 09:11:34 +0200, Martin Kletzander wrote:
>> On Wed, Sep 06, 2023 at 06:34:42PM +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(-)
>
>[...]
>
>> >         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)
>>
>> ... here it only skips the unmount in such case.  That makes sense
>> because for example tmpfs does not have a source path, only the
>> destination and the mount that is not visible in the context here would
>> not only be skipped as well, but the whole starting of a domain would
>> fail.
>>
>> Instead of this patch, you could just conditionally change the
>> VIR_DEBUG() to report what's actually happening.
>
>And we have a handy NULLSTR() macro for that case.
>

NULLSTR(a->b) won't help you if a == NULL, unfortunately :(


>>
>> >             return -1;
>> >
>> > --
>> > 2.34.1
>> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230907/d714c172/attachment.sig>