[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