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

[David Herrmann]

Hi

On Mon, Sep 21, 2015 at 11:54 AM, Paul Osmialowski
<p.osmialowsk@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/