On Thu, Apr 07, 2016 at 02:33:54AM +0300, Dan Jurgens wrote: > From: Daniel Jurgens <danielj@xxxxxxxxxxxx> > > Allocate and free a security context when creating and destroying a QP. > This context is used for controlling access to PKeys. > > When a request is made to modify a QP that changes the port, PKey index, > alternate path port, or alternate path PKey index, check that the QP has > permission for the PKey in the index on the subnet prefix of the port. > If the QP is shared make sure all handles to the QP also have access. > > Store which port and pkey a QP is using. After the reset to init > transition the user can modify the port and pkey independently. This > adds a transactional aspect to modify QP when the port, pkey, or > alternate path change. Permission must be checked for the new settings, > then the modify can be attempted. If the modify fails the old setting > should be restored. > > To keep the security changes isolated a new file is used to hold security > related functionality. > > Signed-off-by: Daniel Jurgens <danielj@xxxxxxxxxxxx> > Reviewed-by: Eli Cohen <eli@xxxxxxxxxxxx> > --- > drivers/infiniband/core/Makefile | 2 +- > drivers/infiniband/core/core_priv.h | 41 ++++ > drivers/infiniband/core/core_security.c | 331 +++++++++++++++++++++++++++++++ We are already in core, there is no need to call files core_XXX. > drivers/infiniband/core/uverbs_cmd.c | 20 ++- > drivers/infiniband/core/verbs.c | 24 +++- > include/rdma/ib_verbs.h | 28 +++- > 6 files changed, 439 insertions(+), 7 deletions(-) > create mode 100644 drivers/infiniband/core/core_security.c > > diff --git a/drivers/infiniband/core/Makefile b/drivers/infiniband/core/Makefile > index f818538..48a4013 100644 > --- a/drivers/infiniband/core/Makefile > +++ b/drivers/infiniband/core/Makefile > @@ -10,7 +10,7 @@ obj-$(CONFIG_INFINIBAND_USER_ACCESS) += ib_uverbs.o ib_ucm.o \ > > ib_core-y := packer.o ud_header.o verbs.o cq.o sysfs.o \ > device.o fmr_pool.o cache.o netlink.o \ > - roce_gid_mgmt.o > + roce_gid_mgmt.o core_security.o > ib_core-$(CONFIG_INFINIBAND_USER_MEM) += umem.o > ib_core-$(CONFIG_INFINIBAND_ON_DEMAND_PAGING) += umem_odp.o umem_rbtree.o > > diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h > index 722b866..27f2fa8 100644 > --- a/drivers/infiniband/core/core_priv.h > +++ b/drivers/infiniband/core/core_priv.h > @@ -140,4 +140,45 @@ static inline bool rdma_is_upper_dev_rcu(struct net_device *dev, > int ib_get_cached_subnet_prefix(struct ib_device *device, > u8 port_num, > u64 *sn_pfx); > + > +#ifdef CONFIG_SECURITY_INFINIBAND > +int ib_security_modify_qp(struct ib_qp *qp, > + struct ib_qp_attr *qp_attr, > + int qp_attr_mask, > + struct ib_udata *udata); > + > +int ib_security_create_qp_security(struct ib_qp *qp); Why do we need xx_SECURITY_xxxxx_SECURITY in name? > +void ib_security_destroy_qp(struct ib_qp_security *sec); > +int ib_security_open_shared_qp(struct ib_qp *qp); > +void ib_security_close_shared_qp(struct ib_qp_security *sec); > +#else > +static inline int ib_security_modify_qp(struct ib_qp *qp, > + struct ib_qp_attr *qp_attr, > + int qp_attr_mask, > + struct ib_udata *udata) > +{ > + return qp->device->modify_qp(qp->real_qp, > + qp_attr, > + qp_attr_mask, > + udata); > +} > + > +static inline int ib_security_create_qp_security(struct ib_qp *qp) > +{ > + return 0; > +} > + > +static inline void ib_security_destroy_qp(struct ib_qp_security *sec) > +{ > +} > + > +static inline int ib_security_open_shared_qp(struct ib_qp *qp) > +{ > + return 0; > +} > + > +static inline void ib_security_close_shared_qp(struct ib_qp_security *sec) > +{ > +} > +#endif > #endif /* _CORE_PRIV_H */ > diff --git a/drivers/infiniband/core/core_security.c b/drivers/infiniband/core/core_security.c > new file mode 100644 > index 0000000..768edea > --- /dev/null > +++ b/drivers/infiniband/core/core_security.c > @@ -0,0 +1,331 @@ > +/* > + * Copyright (c) 2016 Mellanox Technologies Ltd. All rights reserved. > + * > + * This software is available to you under a choice of one of two > + * licenses. You may choose to be licensed under the terms of the GNU > + * General Public License (GPL) Version 2, available from the file > + * COPYING in the main directory of this source tree, or the > + * OpenIB.org BSD license below: > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * - Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the following > + * disclaimer. > + * > + * - Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE > + * SOFTWARE. > + */ > + > +#ifdef CONFIG_SECURITY_INFINIBAND > + > +#include <linux/security.h> > + > +#include <rdma/ib_verbs.h> > +#include <rdma/ib_cache.h> > +#include "core_priv.h" > + > +static int get_pkey_info(struct ib_device *dev, > + u8 port_num, > + u16 pkey_index, > + u64 *subnet_prefix, > + u16 *pkey) > +{ > + int err; > + > + err = ib_get_cached_pkey(dev, port_num, pkey_index, pkey); > + if (err) > + return err; > + > + err = ib_get_cached_subnet_prefix(dev, port_num, subnet_prefix); > + > + return err; > +} > + > +static int enforce_qp_pkey_security(struct ib_device *dev, > + u8 port_num, > + u16 pkey_index, > + struct ib_qp_security *sec) > +{ > + struct ib_qp_security *shared_qp_sec; > + u64 subnet_prefix; > + int err = 0; > + u16 pkey; > + > + err = get_pkey_info(dev, port_num, pkey_index, &subnet_prefix, &pkey); > + if (err) > + return err; > + > + err = security_qp_pkey_access(subnet_prefix, pkey, sec); > + if (err) > + return err; > + > + if (sec->qp == sec->qp->real_qp) { > + /* The caller of this function holds the QP security > + * mutex so this list traversal is safe > + */ Did the comment below pass checkpatch.pl? > + list_for_each_entry(shared_qp_sec, > + &sec->shared_qp_list, > + shared_qp_list) { Is this list always needed to be protected by lock? In general, I prefer to see lock/unlock operations near protected code. Otherwise, it is hard to follow all lock/unlock paths. > + err = security_qp_pkey_access(subnet_prefix, > + pkey, > + shared_qp_sec); > + if (err) > + break; > + } > + } > + return err; > +} > + > +static int check_pkey(const struct ib_qp *qp, > + const struct ib_qp_attr *qp_attr, > + int qp_attr_mask) > +{ > + bool check_pkey = !!(qp_attr_mask & (IB_QP_PKEY_INDEX | IB_QP_PORT)); > + > + return check_pkey && (qp->qp_num != IB_QPT_SMI && > + qp->qp_num != IB_QPT_GSI); IB_QPT_SMI and IB_QPT_GSI are declared as struct ib_qp_type and setted in qp->qp_type and not in qp->qp_num. > +} > + > +static int check_alt_pkey(const struct ib_qp *qp, > + const struct ib_qp_attr *qp_attr, > + int qp_attr_mask) > +{ > + bool check_alt_pkey = !!(qp_attr_mask & IB_QP_ALT_PATH); > + > + return check_alt_pkey && (qp->qp_num != IB_QPT_SMI && > + qp->qp_num != IB_QPT_GSI); > +} The same as above. > + > +static int affects_security_settings(const struct ib_qp *qp, > + const struct ib_qp_attr *qp_attr, > + int qp_attr_mask) > +{ > + return check_pkey(qp, qp_attr, qp_attr_mask) || > + check_alt_pkey(qp, qp_attr, qp_attr_mask); > +} > + > +static void begin_port_pkey_change(struct ib_qp *qp, > + struct ib_port_pkey *pp, > + struct ib_port_pkey *old_pp, > + u8 port_num, > + u16 pkey_index) > +{ > + if (pp->state == IB_PORT_PKEY_NOT_VALID || > + (pkey_index != pp->pkey_index || > + port_num != pp->port_num)) { > + old_pp->pkey_index = pp->pkey_index; > + old_pp->port_num = pp->port_num; > + old_pp->state = pp->state; > + > + pp->port_num = port_num; > + pp->pkey_index = pkey_index; > + pp->state = IB_PORT_PKEY_CHANGING; > + } > +} > + > +static int qp_modify_enforce_security(struct ib_qp *qp, > + const struct ib_qp_attr *qp_attr, > + int qp_attr_mask) > +{ > + struct ib_qp_security *sec = qp->qp_sec; > + int err = 0; > + > + if (check_pkey(qp, qp_attr, qp_attr_mask)) { > + u8 port_num = (qp_attr_mask & IB_QP_PORT) ? > + qp_attr->port_num : > + sec->ports_pkeys.main.port_num; > + > + u16 pkey_index = (qp_attr_mask & IB_QP_PKEY_INDEX) ? > + qp_attr->pkey_index : > + sec->ports_pkeys.main.pkey_index; > + > + err = enforce_qp_pkey_security(qp->device, > + port_num, > + pkey_index, > + sec); > + > + if (err) > + return err; > + > + begin_port_pkey_change(qp, > + &sec->ports_pkeys.main, > + &sec->old_ports_pkeys.main, > + port_num, > + pkey_index); > + } > + > + if (check_alt_pkey(qp, qp_attr, qp_attr_mask)) { > + err = enforce_qp_pkey_security(qp->device, > + qp_attr->alt_port_num, > + qp_attr->alt_pkey_index, > + sec); > + > + if (err) > + return err; > + > + begin_port_pkey_change(qp, > + &sec->ports_pkeys.alt, > + &sec->old_ports_pkeys.alt, > + qp_attr->alt_port_num, > + qp_attr->alt_pkey_index); > + } > + return err; > +} > + > +static void abort_port_pkey_change(struct ib_qp *qp, > + struct ib_port_pkey *pp, > + struct ib_port_pkey *old_pp) > +{ > + if (pp->state == IB_PORT_PKEY_CHANGING) { > + pp->pkey_index = old_pp->pkey_index; > + pp->port_num = old_pp->port_num; > + pp->state = old_pp->state; > + } > +} > + > +static int cleanup_qp_pkey_associations(struct ib_qp *qp, > + bool revert_to_old) > +{ > + struct ib_qp_security *sec = qp->qp_sec; > + > + if (revert_to_old) { > + abort_port_pkey_change(qp, > + &qp->qp_sec->ports_pkeys.main, > + &qp->qp_sec->old_ports_pkeys.main); > + > + abort_port_pkey_change(qp, > + &qp->qp_sec->ports_pkeys.alt, > + &qp->qp_sec->old_ports_pkeys.alt); > + } else { > + if (sec->ports_pkeys.main.state == IB_PORT_PKEY_CHANGING) > + sec->ports_pkeys.main.state = IB_PORT_PKEY_VALID; > + > + if (sec->ports_pkeys.alt.state == IB_PORT_PKEY_CHANGING) > + sec->ports_pkeys.alt.state = IB_PORT_PKEY_VALID; > + } > + > + memset(&sec->old_ports_pkeys, 0, sizeof(sec->old_ports_pkeys)); > + > + return 0; > +} > + > +int ib_security_open_shared_qp(struct ib_qp *qp) > +{ > + struct ib_qp *real_qp = qp->real_qp; > + int err; > + > + err = ib_security_create_qp_security(qp); > + if (err) > + goto out; > + > + mutex_lock(&real_qp->qp_sec->mutex); > + > + if (real_qp->qp_sec->ports_pkeys.main.state != IB_PORT_PKEY_NOT_VALID) > + err = enforce_qp_pkey_security(real_qp->device, > + real_qp->qp_sec->ports_pkeys.main.port_num, > + real_qp->qp_sec->ports_pkeys.main.pkey_index, > + qp->qp_sec); > + if (err) > + goto err; > + > + if (real_qp->qp_sec->ports_pkeys.alt.state != IB_PORT_PKEY_NOT_VALID) > + err = enforce_qp_pkey_security(real_qp->device, > + real_qp->qp_sec->ports_pkeys.alt.port_num, > + real_qp->qp_sec->ports_pkeys.alt.pkey_index, > + qp->qp_sec); > + > + if (err) > + goto err; > + > + if (qp != real_qp) > + list_add(&qp->qp_sec->shared_qp_list, > + &real_qp->qp_sec->shared_qp_list); > +err: > + mutex_unlock(&real_qp->qp_sec->mutex); > + if (err) > + ib_security_destroy_qp(qp->qp_sec); > + > +out: > + return err; > +} > + > +void ib_security_close_shared_qp(struct ib_qp_security *sec) > +{ > + struct ib_qp *real_qp = sec->qp->real_qp; > + > + mutex_lock(&real_qp->qp_sec->mutex); > + list_del(&sec->shared_qp_list); > + mutex_unlock(&real_qp->qp_sec->mutex); > + > + ib_security_destroy_qp(sec); > +} > + > +int ib_security_create_qp_security(struct ib_qp *qp) > +{ > + int err; > + > + qp->qp_sec = kzalloc(sizeof(*qp->qp_sec), GFP_KERNEL); > + if (!qp->qp_sec) > + return -ENOMEM; > + > + qp->qp_sec->qp = qp; > + mutex_init(&qp->qp_sec->mutex); > + INIT_LIST_HEAD(&qp->qp_sec->shared_qp_list); > + err = security_ib_qp_alloc_security(qp->qp_sec); > + if (err) > + kfree(qp->qp_sec); > + > + return err; > +} > +EXPORT_SYMBOL(ib_security_create_qp_security); > + > +void ib_security_destroy_qp(struct ib_qp_security *sec) > +{ > + security_ib_qp_free_security(sec); > + kfree(sec); > +} Did you want to EXPORT_SYMBOL here too? > + > +int ib_security_modify_qp(struct ib_qp *qp, > + struct ib_qp_attr *qp_attr, > + int qp_attr_mask, > + struct ib_udata *udata) > +{ > + int err = 0; > + bool enforce_security = affects_security_settings(qp, > + qp_attr, > + qp_attr_mask); > + > + if (enforce_security) { > + mutex_lock(&qp->qp_sec->mutex); > + > + err = qp_modify_enforce_security(qp, qp_attr, qp_attr_mask); > + } > + > + if (!err) > + err = qp->device->modify_qp(qp->real_qp, > + qp_attr, > + qp_attr_mask, > + udata); > + if (enforce_security) { > + cleanup_qp_pkey_associations(qp, !!err); > + mutex_unlock(&qp->qp_sec->mutex); > + } > + return err; > +} > +EXPORT_SYMBOL(ib_security_modify_qp); > + > +#endif /* CONFIG_SECURITY_INFINIBAND */ > diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c > index 6fdc7ec..6df15ea 100644 > --- a/drivers/infiniband/core/uverbs_cmd.c > +++ b/drivers/infiniband/core/uverbs_cmd.c > @@ -37,7 +37,7 @@ > #include <linux/fs.h> > #include <linux/slab.h> > #include <linux/sched.h> > - > +#include <linux/security.h> > #include <asm/uaccess.h> > > #include "uverbs.h" > @@ -1857,6 +1857,10 @@ static int create_qp(struct ib_uverbs_file *file, > } > > if (cmd->qp_type != IB_QPT_XRC_TGT) { > + ret = ib_security_create_qp_security(qp); > + if (ret) > + goto err_destroy; > + > qp->real_qp = qp; > qp->device = device; > qp->pd = pd; > @@ -2339,10 +2343,18 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file, > ret = ib_resolve_eth_dmac(qp, attr, &cmd.attr_mask); > if (ret) > goto release_qp; > - ret = qp->device->modify_qp(qp, attr, > - modify_qp_mask(qp->qp_type, cmd.attr_mask), &udata); > + > + ret = ib_security_modify_qp(qp, > + attr, > + modify_qp_mask(qp->qp_type, > + cmd.attr_mask), > + &udata); > } else { > - ret = ib_modify_qp(qp, attr, modify_qp_mask(qp->qp_type, cmd.attr_mask)); > + ret = ib_security_modify_qp(qp, > + attr, > + modify_qp_mask(qp->qp_type, > + cmd.attr_mask), > + NULL); > } > > if (ret) > diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c > index 15b8adb..47000ee 100644 > --- a/drivers/infiniband/core/verbs.c > +++ b/drivers/infiniband/core/verbs.c > @@ -44,6 +44,7 @@ > #include <linux/in.h> > #include <linux/in6.h> > #include <net/addrconf.h> > +#include <linux/security.h> > > #include <rdma/ib_verbs.h> > #include <rdma/ib_cache.h> > @@ -681,12 +682,19 @@ static struct ib_qp *__ib_open_qp(struct ib_qp *real_qp, > { > struct ib_qp *qp; > unsigned long flags; > + int err; > > qp = kzalloc(sizeof *qp, GFP_KERNEL); > if (!qp) > return ERR_PTR(-ENOMEM); > > qp->real_qp = real_qp; > + err = ib_security_open_shared_qp(qp); > + if (err) { > + kfree(qp); > + return ERR_PTR(err); > + } > + > atomic_inc(&real_qp->usecnt); > qp->device = real_qp->device; > qp->event_handler = event_handler; > @@ -728,11 +736,16 @@ struct ib_qp *ib_create_qp(struct ib_pd *pd, > { > struct ib_qp *qp, *real_qp; > struct ib_device *device; > + int err; > > device = pd ? pd->device : qp_init_attr->xrcd->device; > qp = device->create_qp(pd, qp_init_attr, NULL); > > if (!IS_ERR(qp)) { > + err = ib_security_create_qp_security(qp); > + if (err) > + goto destroy_qp; > + > qp->device = device; > qp->real_qp = qp; > qp->uobject = NULL; > @@ -780,6 +793,10 @@ struct ib_qp *ib_create_qp(struct ib_pd *pd, > } > > return qp; > + > +destroy_qp: > + ib_destroy_qp(qp); > + return ERR_PTR(err); > } > EXPORT_SYMBOL(ib_create_qp); > > @@ -1180,7 +1197,7 @@ int ib_modify_qp(struct ib_qp *qp, > if (ret) > return ret; > > - return qp->device->modify_qp(qp->real_qp, qp_attr, qp_attr_mask, NULL); > + return ib_security_modify_qp(qp->real_qp, qp_attr, qp_attr_mask, NULL); > } > EXPORT_SYMBOL(ib_modify_qp); > > @@ -1209,6 +1226,7 @@ int ib_close_qp(struct ib_qp *qp) > spin_unlock_irqrestore(&real_qp->device->event_handler_lock, flags); > > atomic_dec(&real_qp->usecnt); > + ib_security_close_shared_qp(qp->qp_sec); > kfree(qp); > > return 0; > @@ -1248,6 +1266,7 @@ int ib_destroy_qp(struct ib_qp *qp) > struct ib_pd *pd; > struct ib_cq *scq, *rcq; > struct ib_srq *srq; > + struct ib_qp_security *sec; > int ret; > > if (atomic_read(&qp->usecnt)) > @@ -1260,6 +1279,7 @@ int ib_destroy_qp(struct ib_qp *qp) > scq = qp->send_cq; > rcq = qp->recv_cq; > srq = qp->srq; > + sec = qp->qp_sec; > > ret = qp->device->destroy_qp(qp); > if (!ret) { > @@ -1271,6 +1291,8 @@ int ib_destroy_qp(struct ib_qp *qp) > atomic_dec(&rcq->usecnt); > if (srq) > atomic_dec(&srq->usecnt); > + if (sec) > + ib_security_destroy_qp(sec); > } > > return ret; > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 870c5ac..f71cb47 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -1416,8 +1416,34 @@ struct ib_srq { > } ext; > }; > > +enum port_pkey_state { > + IB_PORT_PKEY_NOT_VALID = 0, > + IB_PORT_PKEY_VALID = 1, > + IB_PORT_PKEY_CHANGING = 2, > +}; > + > +struct ib_port_pkey { > + enum port_pkey_state state; > + u16 pkey_index; > + u8 port_num; > +}; > + > +struct ib_ports_pkeys { > + struct ib_port_pkey main; > + struct ib_port_pkey alt; > +}; > + > struct ib_qp_security { > - void *q_security; > + struct ib_qp *qp; > + /* Hold this mutex when changing port and pkey settings. */ > + struct mutex mutex; > + struct ib_ports_pkeys ports_pkeys; > + struct ib_ports_pkeys old_ports_pkeys; > + /* A list of all open shared QP handles. Required to enforce security > + * properly for all users of a shared QP. > + */ > + struct list_head shared_qp_list; > + void *q_security; > }; > > struct ib_qp { > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.