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

[David Herrmann]

Hi

On Tue, Nov 11, 2014 at 11:33 AM, Susant Sahani sus...@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)
 ---
  src/core/namespace.c | 44 +---
  1 file changed, 37 insertions(+), 7 deletions(-)

 diff --git a/src/core/namespace.c b/src/core/namespace.c
 index 4bc288d..94a8088 100644
 --- a/src/core/namespace.c
 +++ b/src/core/namespace.c
 @@ -157,14 +157,24 @@ static int mount_dev(BindMount *m) {
  return -errno;

  dev = strappenda(temporary_mount, /dev);
 -mkdir(dev, 0755);
 +r = mkdir(dev, 0755);
 +if (r  0) {
 +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) {
 +r = -errno;
 +goto fail;
 +}
 +
  if (mount(/dev/pts, devpts, NULL, MS_BIND, NULL)  0) {
  r = -errno;
  goto fail;
 @@ -174,7 +184,7 @@ static int mount_dev(BindMount *m) {
  symlink(pts/ptmx, devptmx);

  devshm = strappenda(temporary_mount, /dev/shm);
 -mkdir(devshm, 01777);
 +r = mkdir(devshm, 01777);

This lacks error handling.

  r = mount(/dev/shm, devshm, NULL, MS_BIND, NULL);
  if (r  0) {
  r = -errno;
 @@ -182,15 +192,30 @@ static int mount_dev(BindMount *m) {
  }

  devmqueue = strappenda(temporary_mount, /dev/mqueue);
 -mkdir(devmqueue, 0755);
 +r = mkdir(devmqueue, 0755);
 +if (r  0) {
 +r = -errno;
 +goto fail;
 +}
 +
  mount(/dev/mqueue, devmqueue, NULL, MS_BIND, NULL);

  devkdbus = strappenda(temporary_mount, /dev/kdbus);
 -mkdir(devkdbus, 0755);
 +r = mkdir(devkdbus, 0755);
 +if (r  0) {
 +r = -errno;
 +goto fail;
 +}
 +
  mount(/dev/kdbus, devkdbus, NULL, MS_BIND, NULL);

  devhugepages = strappenda(temporary_mount, /dev/hugepages);
 -mkdir(devhugepages, 0755);
 +r = mkdir(devhugepages, 0755);
 +if (r  0) {
 +r = -errno;
 +goto fail;
 +}
 +
  mount(/dev/hugepages, devhugepages, NULL, MS_BIND, NULL);

  devlog = strappenda(temporary_mount, /dev/log);
 @@ -289,7 +314,12 @@ static int mount_kdbus(BindMount *m) {
  }

  root = strappenda(temporary_mount, /kdbus);
 -mkdir(root, 0755);
 +r = mkdir(root, 0755);
 +if (r  0) {
 +r = -errno;
 +goto fail;
 +}
 +

I also wonder whether we should check errno != EEXIST. Haven't
looked at it in detail, yet.

Thanks
David

  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@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

[Susant Sahani]

On 11/17/2014 03:35 PM, David Herrmann wrote:

Hi

Hi,


On Tue, Nov 11, 2014 at 11:33 AM, Susant Sahani sus...@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)
@@ -289,7 +314,12 @@ static int mount_kdbus(BindMount *m) {
  }

  root = strappenda(temporary_mount, /kdbus);
-mkdir(root, 0755);
+r = mkdir(root, 0755);
+if (r  0) {
+r = -errno;
+goto fail;
+}
+


I also wonder whether we should check errno != EEXIST. Haven't
looked at it in detail, yet.


yes it's better. I'll modify it.

Susant
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

[Susant Sahani]

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)
---
 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [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 sus...@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@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

[Susant Sahani]

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)
---
 src/core/namespace.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/core/namespace.c b/src/core/namespace.c
index eb7f2ad..a202545 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -157,14 +157,14 @@ static int mount_dev(BindMount *m) {
 return -errno;
 
 dev = strappenda(temporary_mount, /dev);
-mkdir(dev, 0755);
+(void)mkdir(dev, 0755);
 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);
+(void)mkdir(devpts, 0755);
 if (mount(/dev/pts, devpts, NULL, MS_BIND, NULL)  0) {
 r = -errno;
 goto fail;
@@ -174,7 +174,7 @@ static int mount_dev(BindMount *m) {
 symlink(pts/ptmx, devptmx);
 
 devshm = strappenda(temporary_mount, /dev/shm);
-mkdir(devshm, 01777);
+(void)mkdir(devshm, 01777);
 r = mount(/dev/shm, devshm, NULL, MS_BIND, NULL);
 if (r  0) {
 r = -errno;
@@ -182,11 +182,11 @@ static int mount_dev(BindMount *m) {
 }
 
 devmqueue = strappenda(temporary_mount, /dev/mqueue);
-mkdir(devmqueue, 0755);
+(void)mkdir(devmqueue, 0755);
 mount(/dev/mqueue, devmqueue, NULL, MS_BIND, NULL);
 
 devhugepages = strappenda(temporary_mount, /dev/hugepages);
-mkdir(devhugepages, 0755);
+(void)mkdir(devhugepages, 0755);
 mount(/dev/hugepages, devhugepages, NULL, MS_BIND, NULL);
 
 devlog = strappenda(temporary_mount, /dev/log);
@@ -282,7 +282,7 @@ static int mount_kdbus(BindMount *m) {
 }
 
 root = strappenda(temporary_mount, /kdbus);
-mkdir(root, 0755);
+(void)mkdir(root, 0755);
 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

[David Herrmann]

Hi

On Mon, Nov 17, 2014 at 11:58 AM, Susant Sahani sus...@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)
 ---
  src/core/namespace.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

Applied!

Thanks
David

 diff --git a/src/core/namespace.c b/src/core/namespace.c
 index eb7f2ad..a202545 100644
 --- a/src/core/namespace.c
 +++ b/src/core/namespace.c
 @@ -157,14 +157,14 @@ static int mount_dev(BindMount *m) {
  return -errno;

  dev = strappenda(temporary_mount, /dev);
 -mkdir(dev, 0755);
 +(void)mkdir(dev, 0755);
  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);
 +(void)mkdir(devpts, 0755);
  if (mount(/dev/pts, devpts, NULL, MS_BIND, NULL)  0) {
  r = -errno;
  goto fail;
 @@ -174,7 +174,7 @@ static int mount_dev(BindMount *m) {
  symlink(pts/ptmx, devptmx);

  devshm = strappenda(temporary_mount, /dev/shm);
 -mkdir(devshm, 01777);
 +(void)mkdir(devshm, 01777);
  r = mount(/dev/shm, devshm, NULL, MS_BIND, NULL);
  if (r  0) {
  r = -errno;
 @@ -182,11 +182,11 @@ static int mount_dev(BindMount *m) {
  }

  devmqueue = strappenda(temporary_mount, /dev/mqueue);
 -mkdir(devmqueue, 0755);
 +(void)mkdir(devmqueue, 0755);
  mount(/dev/mqueue, devmqueue, NULL, MS_BIND, NULL);

  devhugepages = strappenda(temporary_mount, /dev/hugepages);
 -mkdir(devhugepages, 0755);
 +(void)mkdir(devhugepages, 0755);
  mount(/dev/hugepages, devhugepages, NULL, MS_BIND, NULL);

  devlog = strappenda(temporary_mount, /dev/log);
 @@ -282,7 +282,7 @@ static int mount_kdbus(BindMount *m) {
  }

  root = strappenda(temporary_mount, /kdbus);
 -mkdir(root, 0755);
 +(void)mkdir(root, 0755);
  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@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

[Susant Sahani]

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)
---
 src/core/namespace.c | 44 +---
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/src/core/namespace.c b/src/core/namespace.c
index 4bc288d..94a8088 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -157,14 +157,24 @@ static int mount_dev(BindMount *m) {
 return -errno;
 
 dev = strappenda(temporary_mount, /dev);
-mkdir(dev, 0755);
+r = mkdir(dev, 0755);
+if (r  0) {
+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) {
+r = -errno;
+goto fail;
+}
+
 if (mount(/dev/pts, devpts, NULL, MS_BIND, NULL)  0) {
 r = -errno;
 goto fail;
@@ -174,7 +184,7 @@ static int mount_dev(BindMount *m) {
 symlink(pts/ptmx, devptmx);
 
 devshm = strappenda(temporary_mount, /dev/shm);
-mkdir(devshm, 01777);
+r = mkdir(devshm, 01777);
 r = mount(/dev/shm, devshm, NULL, MS_BIND, NULL);
 if (r  0) {
 r = -errno;
@@ -182,15 +192,30 @@ static int mount_dev(BindMount *m) {
 }
 
 devmqueue = strappenda(temporary_mount, /dev/mqueue);
-mkdir(devmqueue, 0755);
+r = mkdir(devmqueue, 0755);
+if (r  0) {
+r = -errno;
+goto fail;
+}
+
 mount(/dev/mqueue, devmqueue, NULL, MS_BIND, NULL);
 
 devkdbus = strappenda(temporary_mount, /dev/kdbus);
-mkdir(devkdbus, 0755);
+r = mkdir(devkdbus, 0755);
+if (r  0) {
+r = -errno;
+goto fail;
+}
+
 mount(/dev/kdbus, devkdbus, NULL, MS_BIND, NULL);
 
 devhugepages = strappenda(temporary_mount, /dev/hugepages);
-mkdir(devhugepages, 0755);
+r = mkdir(devhugepages, 0755);
+if (r  0) {
+r = -errno;
+goto fail;
+}
+
 mount(/dev/hugepages, devhugepages, NULL, MS_BIND, NULL);
 
 devlog = strappenda(temporary_mount, /dev/log);
@@ -289,7 +314,12 @@ static int mount_kdbus(BindMount *m) {
 }
 
 root = strappenda(temporary_mount, /kdbus);
-mkdir(root, 0755);
+r = mkdir(root, 0755);
+if (r  0) {
+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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel