[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8738a6w6kv.fsf@x220.int.ebiederm.org>
Date: Wed, 29 Oct 2014 20:59:44 -0700
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
john.stultz@...aro.org, arnd@...db.de, tj@...nel.org,
marcel@...tmann.org, desrt@...rt.ca, hadess@...ess.net,
dh.herrmann@...il.com, tixxdz@...ndz.org,
simon.mcvittie@...labora.co.uk, daniel@...que.org,
alban.crequy@...labora.co.uk, javier.martinez@...labora.co.uk,
teg@...m.no, Andy Lutomirski <luto@...capital.net>
Subject: Re: kdbus: add code for buses, domains and endpoints
Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes:
The way capabilities are checked in this patch make me very nervous.
We are not checking permissions at open time. Every other location
of calling capable on file like objects has been show to be suceptible
to file descriptor pass attacks.
> See Documentation/kdbus.txt for more details.
>
> Signed-off-by: Daniel Mack <daniel@...que.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
> diff --git a/drivers/misc/kdbus/bus.c b/drivers/misc/kdbus/bus.c
> new file mode 100644
> index 000000000000..6dcaf22f5d59
> --- /dev/null
> +++ b/drivers/misc/kdbus/bus.c
> @@ -0,0 +1,450 @@
> +/**
> + * kdbus_bus_cred_is_privileged() - check whether the given credentials in
> + * combination with the capabilities of the
> + * current thead are privileged on the bus
> + * @bus: The bus to check
> + * @cred: The credentials to match
> + *
> + * Return: true if the credentials are privileged, otherwise false.
> + */
> +bool kdbus_bus_cred_is_privileged(const struct kdbus_bus *bus,
> + const struct cred *cred)
> +{
> + /* Capabilities are *ALWAYS* tested against the current thread, they're
> + * never remembered from conn-credentials. */
> + if (ns_capable(&init_user_ns, CAP_IPC_OWNER))
> + return true;
> +
> + return uid_eq(bus->uid_owner, cred->fsuid);
> +}
> +
> +/**
> + * kdbus_bus_uid_is_privileged() - check whether the current user is a
> + * priviledged bus user
> + * @bus: The bus to check
> + *
> + * Return: true if the current user has CAP_IPC_OWNER capabilities, or
> + * if it has the same UID as the user that created the bus. Otherwise,
> + * false is returned.
> + */
> +bool kdbus_bus_uid_is_privileged(const struct kdbus_bus *bus)
> +{
> + return kdbus_bus_cred_is_privileged(bus, current_cred());
> +}
> +/**
> + * kdbus_bus_new() - create a new bus
> + * @domain: The domain to work on
> + * @make: Pointer to a struct kdbus_cmd_make containing the
> + * details for the bus creation
> + * @name: Name of the bus
> + * @bloom: Bloom parameters for this bus
> + * @mode: The access mode for the device node
> + * @uid: The uid of the device node
> + * @gid: The gid of the device node
> + * @bus: Pointer to a reference where the new bus is stored
> + *
> + * This function will allocate a new kdbus_bus and link it to the given
> + * domain.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int kdbus_bus_new(struct kdbus_domain *domain,
> + const struct kdbus_cmd_make *make,
> + const char *name,
> + const struct kdbus_bloom_parameter *bloom,
> + umode_t mode, kuid_t uid, kgid_t gid,
> + struct kdbus_bus **bus)
> +{
[snip]
> +
> + if (!capable(CAP_IPC_OWNER) &&
> + atomic_inc_return(&b->user->buses) > KDBUS_USER_MAX_BUSES) {
> + atomic_dec(&b->user->buses);
> + ret = -EMFILE;
> + goto exit_unref_user_unlock;
> + }
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists