[systemd-devel] [PATCH 1/2] namespace:Unchecked return value from library

[David Herrmann]

Hi

On Mon, Nov 17, 2014 at 11:37 AM, Susant Sahani <susant at redhat.com> wrote:
> fix:
>  CID 1237553 (#1 of 6): Unchecked return value from library
> (CHECKED_RETURN
>
> CID 1237553 (#3 of 6): Unchecked return value from library
> (CHECKED_RETURN)
>
> CID 1237553 (#4 of 6): Unchecked return value from library
> (CHECKED_RETURN)
>
> CID 1237553 (#5 of 6): Unchecked return value from library
> (CHECKED_RETURN
>
> CID 1237553 (#6 of 6): Unchecked return value from library
> (CHECKED_RETURN)
> ---

Looking into mount_dev() more closely, I noticed this is probably not
needed at all. I mean, we create a temporary directory and then mount
everything in there. See mkdtemp(). If mkdir() fails, then mount()
will fail too. No-one else can mess with us as they need to be root to
do anything bad to our temp-dir (and if they're root, we're screwed
anyway).

So I guess we can just ignore all the errors. I'd be fine with
pre-fixing them with "(void)", or dropping EEXIST again (sorry!) and
doing normal error-checking.

Thanks
David

>  src/core/namespace.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/src/core/namespace.c b/src/core/namespace.c
> index eb7f2ad..db99e88 100644
> --- a/src/core/namespace.c
> +++ b/src/core/namespace.c
> @@ -157,14 +157,26 @@ static int mount_dev(BindMount *m) {
>                  return -errno;
>
>          dev = strappenda(temporary_mount, "/dev");
> -        mkdir(dev, 0755);
> +
> +        r = mkdir(dev, 0755);
> +        if (r < 0 && errno != EEXIST) {
> +                r = -errno;
> +                goto fail;
> +        }
> +
>          if (mount("tmpfs", dev, "tmpfs", MS_NOSUID|MS_STRICTATIME, "mode=755") < 0) {
>                  r = -errno;
>                  goto fail;
>          }
>
>          devpts = strappenda(temporary_mount, "/dev/pts");
> -        mkdir(devpts, 0755);
> +
> +        r = mkdir(devpts, 0755);
> +        if (r < 0 && errno != EEXIST) {
> +                r = -errno;
> +                goto fail;
> +        }
> +
>          if (mount("/dev/pts", devpts, NULL, MS_BIND, NULL) < 0) {
>                  r = -errno;
>                  goto fail;
> @@ -174,7 +186,13 @@ static int mount_dev(BindMount *m) {
>          symlink("pts/ptmx", devptmx);
>
>          devshm = strappenda(temporary_mount, "/dev/shm");
> -        mkdir(devshm, 01777);
> +
> +        r = mkdir(devshm, 01777);
> +        if (r < 0 && errno != EEXIST) {
> +                r = -errno;
> +                goto fail;
> +        }
> +
>          r = mount("/dev/shm", devshm, NULL, MS_BIND, NULL);
>          if (r < 0) {
>                  r = -errno;
> @@ -182,11 +200,23 @@ static int mount_dev(BindMount *m) {
>          }
>
>          devmqueue = strappenda(temporary_mount, "/dev/mqueue");
> -        mkdir(devmqueue, 0755);
> +
> +        r = mkdir(devmqueue, 0755);
> +        if (r < 0 && errno != EEXIST) {
> +                r = -errno;
> +                goto fail;
> +        }
> +
>          mount("/dev/mqueue", devmqueue, NULL, MS_BIND, NULL);
>
>          devhugepages = strappenda(temporary_mount, "/dev/hugepages");
> -        mkdir(devhugepages, 0755);
> +
> +        r = mkdir(devhugepages, 0755);
> +        if (r < 0 && errno != EEXIST) {
> +                r = -errno;
> +                goto fail;
> +        }
> +
>          mount("/dev/hugepages", devhugepages, NULL, MS_BIND, NULL);
>
>          devlog = strappenda(temporary_mount, "/dev/log");
> @@ -282,7 +312,13 @@ static int mount_kdbus(BindMount *m) {
>          }
>
>          root = strappenda(temporary_mount, "/kdbus");
> -        mkdir(root, 0755);
> +
> +        r = mkdir(root, 0755);
> +        if (r < 0 && errno != EEXIST) {
> +                r = -errno;
> +                goto fail;
> +        }
> +
>          if (mount("tmpfs", root, "tmpfs", MS_NOSUID|MS_STRICTATIME, "mode=777") < 0) {
>                  r = -errno;
>                  goto fail;
> --
> 2.1.0
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel