Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1229168 > unrolled thread

[RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors

Started byPaul Osmialowski <p.osmialowsk@samsung.com>
First post2015-09-21 12:00 +0200
Last post2015-09-21 12:20 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [RFC v2] kdbus: use LSM hooks to restrict ability to send file  descriptors Paul Osmialowski <p.osmialowsk@samsung.com> - 2015-09-21 12:00 +0200
    Re: [RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors David Herrmann <dh.herrmann@gmail.com> - 2015-09-21 12:20 +0200

#1229168 — [RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors

FromPaul Osmialowski <p.osmialowsk@samsung.com>
Date2015-09-21 12:00 +0200
Subject[RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors
Message-ID<qb638-33B-21@gated-at.bofh.it>
The goal of this patch is to reproduce on kdbus the same behavior
that is expressed by Unix Domain Sockets when it comes to restricting
ability to pass opened file descriptors.

Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 ipc/kdbus/message.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
index ae565cd..7f8aa35 100644
--- a/ipc/kdbus/message.c
+++ b/ipc/kdbus/message.c
@@ -24,6 +24,7 @@
 #include <linux/sizes.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
+#include <linux/security.h>
 #include <net/sock.h>
 
 #include "bus.h"
@@ -150,13 +151,19 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
 		for (i = 0; i < gaps->n_fds; ++i) {
 			int fd;
 
-			fd = get_unused_fd_flags(O_CLOEXEC);
-			if (fd < 0)
-				incomplete_fds = true;
-
 			WARN_ON(!gaps->fd_files[i]);
 
-			fds[n_fds++] = fd < 0 ? -1 : fd;
+			if (gaps->fd_files[i] &&
+				    security_file_receive(gaps->fd_files[i])) {
+				incomplete_fds = true;
+				fds[n_fds++] = -1;
+			} else {
+				fd = get_unused_fd_flags(O_CLOEXEC);
+				if (fd < 0)
+					incomplete_fds = true;
+
+				fds[n_fds++] = fd < 0 ? -1 : fd;
+			}
 		}
 
 		/*
@@ -178,6 +185,16 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
 	for (i = 0; i < gaps->n_memfds; ++i) {
 		int memfd;
 
+		WARN_ON(!gaps->memfd_offsets[i]);
+		WARN_ON(!gaps->memfd_files[i]);
+
+		if (gaps->memfd_files[i] &&
+			    security_file_receive(gaps->memfd_files[i])) {
+			incomplete_fds = true;
+			fds[n_fds++] = -1;
+			continue;
+		}
+
 		memfd = get_unused_fd_flags(O_CLOEXEC);
 		if (memfd < 0) {
 			incomplete_fds = true;
@@ -193,10 +210,6 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
 		 * as usually there is no need to send more than one memfd per
 		 * message.
 		 */
-
-		WARN_ON(!gaps->memfd_offsets[i]);
-		WARN_ON(!gaps->memfd_files[i]);
-
 		kvec.iov_base = &memfd;
 		kvec.iov_len = sizeof(memfd);
 		ret = kdbus_pool_slice_copy_kvec(slice, gaps->memfd_offsets[i],
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1229192 — Re: [RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors

FromDavid Herrmann <dh.herrmann@gmail.com>
Date2015-09-21 12:20 +0200
SubjectRe: [RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors
Message-ID<qb6mu-3FW-41@gated-at.bofh.it>
In reply to#1229168
Hi

On Mon, Sep 21, 2015 at 11:54 AM, Paul Osmialowski
<p.osmialowsk@samsung.com> wrote:
> The goal of this patch is to reproduce on kdbus the same behavior
> that is expressed by Unix Domain Sockets when it comes to restricting
> ability to pass opened file descriptors.
>
> Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
> ---
>  ipc/kdbus/message.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/ipc/kdbus/message.c b/ipc/kdbus/message.c
> index ae565cd..7f8aa35 100644
> --- a/ipc/kdbus/message.c
> +++ b/ipc/kdbus/message.c
> @@ -24,6 +24,7 @@
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
> +#include <linux/security.h>
>  #include <net/sock.h>
>
>  #include "bus.h"
> @@ -150,13 +151,19 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
>                 for (i = 0; i < gaps->n_fds; ++i) {
>                         int fd;
>
> -                       fd = get_unused_fd_flags(O_CLOEXEC);
> -                       if (fd < 0)
> -                               incomplete_fds = true;
> -
>                         WARN_ON(!gaps->fd_files[i]);
>
> -                       fds[n_fds++] = fd < 0 ? -1 : fd;
> +                       if (gaps->fd_files[i] &&

Please drop the WARN_ON above and this condition. It was fine before,
but here it just makes the code more complex, unnecessarily.

> +                                   security_file_receive(gaps->fd_files[i])) {
> +                               incomplete_fds = true;
> +                               fds[n_fds++] = -1;
> +                       } else {
> +                               fd = get_unused_fd_flags(O_CLOEXEC);
> +                               if (fd < 0)
> +                                       incomplete_fds = true;
> +
> +                               fds[n_fds++] = fd < 0 ? -1 : fd;
> +                       }
>                 }
>
>                 /*
> @@ -178,6 +185,16 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
>         for (i = 0; i < gaps->n_memfds; ++i) {
>                 int memfd;
>
> +               WARN_ON(!gaps->memfd_offsets[i]);
> +               WARN_ON(!gaps->memfd_files[i]);
> +
> +               if (gaps->memfd_files[i] &&

Same as above, just drop the WARN_ON and this condition.

> +                           security_file_receive(gaps->memfd_files[i])) {
> +                       incomplete_fds = true;
> +                       fds[n_fds++] = -1;
> +                       continue;
> +               }
> +

I don't see the point in protecting transmission of memfds. They're
treated as inline data. But I will not object it.

Patch looks good to me.

Thanks
David

>                 memfd = get_unused_fd_flags(O_CLOEXEC);
>                 if (memfd < 0) {
>                         incomplete_fds = true;
> @@ -193,10 +210,6 @@ int kdbus_gaps_install(struct kdbus_gaps *gaps, struct kdbus_pool_slice *slice,
>                  * as usually there is no need to send more than one memfd per
>                  * message.
>                  */
> -
> -               WARN_ON(!gaps->memfd_offsets[i]);
> -               WARN_ON(!gaps->memfd_files[i]);
> -
>                 kvec.iov_base = &memfd;
>                 kvec.iov_len = sizeof(memfd);
>                 ret = kdbus_pool_slice_copy_kvec(slice, gaps->memfd_offsets[i],
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web