linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors
@ 2015-09-21  9:54 Paul Osmialowski
  2015-09-21 10:11 ` David Herrmann
  0 siblings, 1 reply; 2+ messages in thread
From: Paul Osmialowski @ 2015-09-21  9:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
	Paul Moore, linux-kernel, linux-security-module
  Cc: Paul Osmialowski, Lukasz Pawelczyk

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors
  2015-09-21  9:54 [RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors Paul Osmialowski
@ 2015-09-21 10:11 ` David Herrmann
  0 siblings, 0 replies; 2+ messages in thread
From: David Herrmann @ 2015-09-21 10:11 UTC (permalink / raw)
  To: Paul Osmialowski
  Cc: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni,
	Paul Moore, linux-kernel, LSM, Lukasz Pawelczyk

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
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-09-21 10:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-21  9:54 [RFC v2] kdbus: use LSM hooks to restrict ability to send file descriptors Paul Osmialowski
2015-09-21 10:11 ` David Herrmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).