[CRIU] [PATCH v8 00/15] AutoFS mounts migration support

Andrew Vagin avagin at virtuozzo.com
Wed Mar 23 16:03:59 PDT 2016


Stas, here are four issues which were reported by Coverirty:

324static int autofs_dump_entry(struct mount_info *pm, AutofsEntry
*entry)
325{
326        struct cr_img *img;
327        int ret = -1;
328
329        img = open_image(CR_FD_AUTOFS, O_DUMP, pm->k_dev);
   	1. Condition img, taking false branch
   	2. var_compare_op: Comparing img to null implies that img might
be null.
330        if (img)
331                ret = pb_write_one(img, entry, PB_AUTOFS);
   	
CID 158925 (#1 of 1): Dereference after null check (FORWARD_NULL)
3. var_deref_model: Passing null pointer img to close_image, which
dereferences it. [show details]
332        close_image(img);
333        return ret;
334}


337int autofs_dump(struct mount_info *pm)
338{
339        AutofsEntry *entry;
340
   	1. alloc_fn: Storage is returned from allocation function
malloc.
   	2. var_assign: Assigning: ___p = storage returned from
malloc(72UL).
   	3. Condition !___p, taking false branch
   	4. leaked_storage: Variable ___p going out of scope leaks the
storage it points to.
   	5. var_assign: Assigning: entry = ({...}).
341        entry = xmalloc(sizeof(*entry));
   	6. Condition !entry, taking false branch
342        if (!entry)
343                return -1;
   	7. noescape: Resource entry is not freed or pointed-to in
autofs_entry__init. [show details]
344        autofs_entry__init(entry);
345
   	8. noescape: Resource entry is not freed or pointed-to in
autofs_create_entry. [show details]
   	9. Condition autofs_create_entry(pm, entry), taking true branch
346        if (autofs_create_entry(pm, entry))
   	
CID 158926 (#1 of 1): Resource leak (RESOURCE_LEAK)
10. leaked_storage: Variable entry going out of scope leaks the storage
it points to.
347                return -1;
348
349        return autofs_dump_entry(pm, entry);
350}

int autofs_mount(struct mount_info *mi, const char *source, const
822                 char *filesystemtype, unsigned long mountflags)
823{
824        AutofsEntry *entry;
825        autofs_info_t *info;
826        char *opts, *mode;
827        int control_pipe[2], ret = -1;
828        struct stat buf;
829
830        if (autofs_restore_entry(mi, &entry) < 0)
831                return -1;
832
833        if (pipe(control_pipe) < 0) {
834                pr_perror("Can't create pipe");
835                return -1;
836        }
837
838        mode = "direct";
839        if (entry->mode == AUTOFS_MODE_INDIRECT)
840                mode = "indirect";
   	1. Starting defect path here.
   	2. Condition entry->mode == 2, taking true branch
841        if (entry->mode == AUTOFS_MODE_OFFSET)
842                mode = "offset";
843
844        opts = xsprintf("fd=%d,pgrp=%d,minproto=%d,maxproto=%d,%s",
845                        control_pipe[1], getpgrp(), entry->minproto,
846                        entry->maxproto, mode);
   	3. Condition opts, taking true branch
   	4. Condition entry->has_uid, taking true branch
847        if (opts && entry->has_uid)
848                opts = xstrcat(opts, ",uid=%d", entry->uid);
   	5. Condition opts, taking true branch
   	6. Condition entry->has_gid, taking true branch
849        if (opts && entry->has_gid)
850                opts = xstrcat(opts, ",gid=%d", entry->gid);
   	7. Condition !opts, taking false branch
851        if (!opts) {
852                pr_err("Failed to create options string\n");
853                goto close_pipe;
854        }
855
856        pr_info("autofs: mounting to %s with options: \"%s\"\n",
857                        mi->mountpoint, opts);
858
   	8. Condition mount(source, mi->mountpoint, filesystemtype,
mountflags, opts) < 0, taking false branch
859        if (mount(source, mi->mountpoint, filesystemtype, mountflags,
opts) < 0) {
860                pr_perror("Failed to mount autofs to %s",
mi->mountpoint);
861                goto close_pipe;
862        }
863
   	9. alloc_fn: Storage is returned from allocation function
malloc.
   	10. var_assign: Assigning: ___p = storage returned from
malloc(120UL).
   	11. Condition !___p, taking false branch
   	12. leaked_storage: Variable ___p going out of scope leaks the
storage it points to.
   	13. var_assign: Assigning: info = ({...}).
864        info = xmalloc(sizeof(*info));
   	14. Condition !info, taking false branch
865        if (!info)
866                goto umount;
867        info->entry = entry;
868
869        /* We need autofs dev_id to be able to open direct mount
point.
870         * But we can't call stat in autofs_add_mount_info(), because
autofs
871         * mount can be overmounted. Thus we have to call it here.
But shared
872         * data is not ready yet. So, let's put in on mi->private and
copy to
873         * shared data in autofs_add_mount_info().
874         */
   	CID 158928: Time of check time of use (TOCTOU) [select issue]
   	15. Condition stat(mi->mountpoint, &buf) < 0, taking false
branch
875        if (stat(mi->mountpoint, &buf) < 0) {
876                pr_perror("Failed to stat %s", mi->mountpoint);
877                goto umount;
878        }
879        info->mnt_dev = buf.st_dev;
880
881        /* We need to create dentries for nested mounts */
882        ret = autofs_populate_mount(mi, entry);
   	16. Condition ret < 0, taking false branch
883        if (ret < 0)
884                goto umount;
885
886        /* In case of catatonic mounts all we need as the function
call below */
887        ret = autofs_post_mount(mi->mountpoint, buf.st_dev,
entry->timeout);
   	17. Condition ret < 0, taking false branch
888        if (ret < 0)
889                goto umount;
890
891        /* Otherwise we have to add shared object creation callback
*/
   	18. Condition entry->fd != -1, taking true branch
892        if (entry->fd != AUTOFS_CATATONIC_FD) {
893                ret = add_post_prepare_cb(autofs_add_mount_info, mi);
   	19. Condition ret < 0, taking true branch
894                if (ret < 0)
   	20. Jumping to label umount
895                        goto umount;
896        }
897
898        mi->private = info;
899
900close_pipe:
901        close(control_pipe[1]);
902        close(control_pipe[0]);
   	
CID 158927 (#1 of 1): Resource leak (RESOURCE_LEAK)
23. leaked_storage: Variable info going out of scope leaks the storage
it points to.
903        return ret;
904
905umount:
   	21. Condition umount(mi->mountpoint) < 0, taking true branch
906        if (umount(mi->mountpoint) < 0)
907                pr_perror("Failed to umount %s", mi->mountpoint);
   	22. Jumping to label close_pipe
908        goto close_pipe;
909}



821int autofs_mount(struct mount_info *mi, const char *source, const
822                 char *filesystemtype, unsigned long mountflags)
823{
824        AutofsEntry *entry;
825        autofs_info_t *info;
826        char *opts, *mode;
827        int control_pipe[2], ret = -1;
828        struct stat buf;
829
   	1. Condition autofs_restore_entry(mi, &entry) < 0, taking false
branch
830        if (autofs_restore_entry(mi, &entry) < 0)
831                return -1;
832
   	2. Condition pipe(control_pipe) < 0, taking false branch
833        if (pipe(control_pipe) < 0) {
834                pr_perror("Can't create pipe");
835                return -1;
836        }
837
838        mode = "direct";
   	3. Condition entry->mode == 1, taking true branch
839        if (entry->mode == AUTOFS_MODE_INDIRECT)
840                mode = "indirect";
   	4. Condition entry->mode == 2, taking false branch
841        if (entry->mode == AUTOFS_MODE_OFFSET)
842                mode = "offset";
843
844        opts = xsprintf("fd=%d,pgrp=%d,minproto=%d,maxproto=%d,%s",
845                        control_pipe[1], getpgrp(), entry->minproto,
846                        entry->maxproto, mode);
   	5. Condition opts, taking true branch
   	6. Condition entry->has_uid, taking true branch
847        if (opts && entry->has_uid)
848                opts = xstrcat(opts, ",uid=%d", entry->uid);
   	7. Condition opts, taking true branch
   	8. Condition entry->has_gid, taking true branch
849        if (opts && entry->has_gid)
850                opts = xstrcat(opts, ",gid=%d", entry->gid);
   	9. Condition !opts, taking false branch
851        if (!opts) {
852                pr_err("Failed to create options string\n");
853                goto close_pipe;
854        }
855
856        pr_info("autofs: mounting to %s with options: \"%s\"\n",
857                        mi->mountpoint, opts);
858
   	10. Condition mount(source, mi->mountpoint, filesystemtype,
mountflags, opts) < 0, taking false branch
859        if (mount(source, mi->mountpoint, filesystemtype, mountflags,
opts) < 0) {
860                pr_perror("Failed to mount autofs to %s",
mi->mountpoint);
861                goto close_pipe;
862        }
863
   	11. Condition !___p, taking false branch
864        info = xmalloc(sizeof(*info));
   	12. Condition !info, taking false branch
865        if (!info)
866                goto umount;
867        info->entry = entry;
868
869        /* We need autofs dev_id to be able to open direct mount
point.
870         * But we can't call stat in autofs_add_mount_info(), because
autofs
871         * mount can be overmounted. Thus we have to call it here.
But shared
872         * data is not ready yet. So, let's put in on mi->private and
copy to
873         * shared data in autofs_add_mount_info().
874         */
   	
CID 158928 (#1 of 1): Time of check time of use (TOCTOU)
13. fs_check_call: Calling function stat to perform check on
mi->mountpoint.
   	14. Condition stat(mi->mountpoint, &buf) < 0, taking true branch
875        if (stat(mi->mountpoint, &buf) < 0) {
876                pr_perror("Failed to stat %s", mi->mountpoint);
   	15. Jumping to label umount
877                goto umount;
878        }
879        info->mnt_dev = buf.st_dev;
880
881        /* We need to create dentries for nested mounts */
882        ret = autofs_populate_mount(mi, entry);
883        if (ret < 0)
884                goto umount;
885
886        /* In case of catatonic mounts all we need as the function
call below */
887        ret = autofs_post_mount(mi->mountpoint, buf.st_dev,
entry->timeout);
888        if (ret < 0)
889                goto umount;
890
891        /* Otherwise we have to add shared object creation callback
*/
892        if (entry->fd != AUTOFS_CATATONIC_FD) {
893                ret = add_post_prepare_cb(autofs_add_mount_info, mi);
894                if (ret < 0)
895                        goto umount;
896        }
897
898        mi->private = info;
899
900close_pipe:
901        close(control_pipe[1]);
902        close(control_pipe[0]);
   	CID 158927: Resource leak (RESOURCE_LEAK) [select issue]
903        return ret;
904
905umount:
   	16. toctou: Calling function umount that uses mi->mountpoint
after a check function. This can cause a time-of-check, time-of-use race
condition.
906        if (umount(mi->mountpoint) < 0)
907                pr_perror("Failed to umount %s", mi->mountpoint);
908        goto close_pipe;
909}

On Wed, Mar 16, 2016 at 04:16:49PM +0300, Stanislav Kinsburskiy wrote:
> This patch set adds suport for AutoFS mount migration, including:
> 1) Direct mounts
> 2) Indirect mounts
> 3) Catatonic mounts
> 
> Note: systemd AutoFS logic breaks with this migration, because device value of
> new mount differs to the one, which systemd collected, when it was created the
> mount. Because of it systemd ignors any requests from kernel.
> 
> v8:
> 1) find_unused_fd() was optimized: if hint_fd is -1, last used fd + 1 is
> returned without loop iteration.
> 2) Patch "mount: copy private field to "bind mount" mount_info structure" was
> dropped as obsolete.
> 3) Some cleanup in functions naming.
> 4) Feature "autofs" was added to test descriptor.
> 
> v7:
> 1) Rebased on criu-dev branch
> 2) test attached
> 
> v6: indirect mounts dentries are now created within autofs code on autofs
> mount.
> 
> v5: this version has significant changes in comparison to previous one:
> 1) Autofs data is migrated via AutofsEntry structure, which is nested to
> MntEntry structure.
> 2) Dump stage is now only converts real pgrp into virtual one. All the collect
> and control pipe check is done in "parse" stage.
> 3) All the autofs pipes are collected into a list. This list is checked on
> packetized pipe dump to distinguish between autofs and other pipes (which are
> still not supported).
> 4) Control pipe restore is now done by creating another pipe fd with custom
> post_open callback instead of artificial files.
> 5) New list of objects and actors was introduced to create shared autofs data
> after both mounts and shrared objects are restored.
> 6) Timeout restore and making mount catatonic is done right after mount pint
> is created. It allows to avoid of autofs data objects creation and post_open
> callback calling in case of catatonic mounts.
> 
> v4: mount info is now transfered in deducated autofs images.
> 
> v3: fixed write pipe end restore, when it was closed.
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu


More information about the CRIU mailing list