netdev - Re: [PATCH rdma-next 01/12] RDMA/uverbs: Protect from attempts to create flows on unsupported QP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Jun 2018 15:14:40 -0600
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     Doug Ledford <dledford@...hat.com>,
        Leon Romanovsky <leonro@...lanox.com>,
        RDMA mailing list <linux-rdma@...r.kernel.org>,
        Hadar Hen Zion <hadarh@...lanox.com>,
        Matan Barak <matanb@...lanox.com>,
        Michael J Ruhl <michael.j.ruhl@...el.com>,
        Noa Osherovich <noaos@...lanox.com>,
        Raed Salem <raeds@...lanox.com>,
        Yishai Hadas <yishaih@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        linux-netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH rdma-next 01/12] RDMA/uverbs: Protect from attempts to
 create flows on unsupported QP

On Sun, Jun 24, 2018 at 11:23:42AM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@...lanox.com>
> 
> Flows can be created on UD and RAW_PACKET QP types. Attempts to provide
> other QP types as an input causes to various unpredictable failures.
> 
> The reason to it that in order to support all various types (e.g. XRC),
> we are supposed to use real_qp handle and not qp handle and give to
> driver/FW to fail such (XRC) flows. Being valuable solution, the simpler
> and safer variant is to ban all QP types except UD and RAW_PACKET,
> instead of relying on driver/FW.
> 
> Cc: <stable@...r.kernel.org> # 3.11
> Fixes: 436f2ad05a0b ("IB/core: Export ib_create/destroy_flow through uverbs")
> Cc: syzkaller <syzkaller@...glegroups.com>
> Reported-by: Noa Osherovich <noaos@...lanox.com>
> Signed-off-by: Leon Romanovsky <leonro@...lanox.com>
> ---
>  drivers/infiniband/core/uverbs_cmd.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index 779892b63729..c842a9423fbf 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -3553,14 +3553,20 @@ int ib_uverbs_ex_create_flow(struct ib_uverbs_file *file,
>  		goto err_free_attr;
>  	}
>  
> -	qp = uobj_get_obj_read(qp, UVERBS_OBJECT_QP, cmd.qp_handle, file->ucontext);
> +	qp = uobj_get_obj_read(qp, UVERBS_OBJECT_QP, cmd.qp_handle,
> +			       file->ucontext);

This hunk is just whitespace changing

>  	if (!qp) {
>  		err = -EINVAL;
>  		goto err_uobj;
>  	}
>  
> +	if (qp->qp_type != IB_QPT_UD && qp->qp_type != IB_QPT_RAW_PACKET) {
> +		err = -EINVAL;
> +		goto err_put;
> +	}
> +
>  	flow_attr = kzalloc(struct_size(flow_attr, flows,
> -				cmd.flow_attr.num_of_specs), GFP_KERNEL);
> +					cmd.flow_attr.num_of_specs), GFP_KERNEL);

Same here.

I dropped the two hunks and applied this to for-rc since it has
stable tags.

Jason