* [PATCH rdma-rc] RDMA: Verify port when creating flow rule
@ 2021-06-08 5:12 Leon Romanovsky
2021-06-08 20:09 ` Jason Gunthorpe
0 siblings, 1 reply; 5+ messages in thread
From: Leon Romanovsky @ 2021-06-08 5:12 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: Maor Gottlieb, linux-rdma, Mark Bloch
From: Maor Gottlieb <maorg@nvidia.com>
Validate port value provided by the user and with that remove no
longer needed validation by the driver.
The missing check in the mlx5_ib driver could cause to the below oops.
Call trace:
_create_flow_rule+0x2d4/0xf28 [mlx5_ib]
mlx5_ib_create_flow+0x2d0/0x5b0 [mlx5_ib]
ib_uverbs_ex_create_flow+0x4cc/0x624 [ib_uverbs]
ib_uverbs_handler_UVERBS_METHOD_INVOKE_WRITE+0xd4/0x150 [ib_uverbs]
ib_uverbs_cmd_verbs.isra.7+0xb28/0xc50 [ib_uverbs]
ib_uverbs_ioctl+0x158/0x1d0 [ib_uverbs]
do_vfs_ioctl+0xd0/0xaf0
ksys_ioctl+0x84/0xb4
__arm64_sys_ioctl+0x28/0xc4
el0_svc_common.constprop.3+0xa4/0x254
el0_svc_handler+0x84/0xa0
el0_svc+0x10/0x26c
Code: b9401260 f9615681 51000400 8b001c20 (f9403c1a)
---[ end trace 1b5ffb34e3a14d2b ]---
Fixes: 436f2ad05a0b ("IB/core: Export ib_create/destroy_flow through uverbs")
Reviewed-by: Mark Bloch <markb@mellanox.com>
Signed-off-by: Maor Gottlieb <maorg@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
drivers/infiniband/core/uverbs_cmd.c | 8 ++++++++
drivers/infiniband/hw/mlx4/main.c | 3 ---
drivers/infiniband/hw/mlx5/fs.c | 5 ++---
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index d5e15a8c870d..fe81ddfeb165 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -3188,6 +3188,7 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
struct ib_qp *qp;
struct ib_uflow_resources *uflow_res;
struct ib_uverbs_flow_spec_hdr *kern_spec;
+ struct ib_ucontext *ucontext;
struct uverbs_req_iter iter;
int err;
void *ib_spec;
@@ -3198,6 +3199,13 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
if (err)
return err;
+ ucontext = ib_uverbs_get_ucontext(attrs);
+ if (IS_ERR(ucontext))
+ return PTR_ERR(ucontext);
+
+ if (!rdma_is_port_valid(ucontext->device, cmd.flow_attr.port))
+ return -EINVAL;
+
if (cmd.comp_mask)
return -EINVAL;
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 16704262fc3a..230a6ae0ab5a 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -1699,9 +1699,6 @@ static struct ib_flow *mlx4_ib_create_flow(struct ib_qp *qp,
struct mlx4_dev *dev = (to_mdev(qp->device))->dev;
int is_bonded = mlx4_is_bonded(dev);
- if (!rdma_is_port_valid(qp->device, flow_attr->port))
- return ERR_PTR(-EINVAL);
-
if (flow_attr->flags & ~IB_FLOW_ATTR_FLAGS_DONT_TRAP)
return ERR_PTR(-EOPNOTSUPP);
diff --git a/drivers/infiniband/hw/mlx5/fs.c b/drivers/infiniband/hw/mlx5/fs.c
index 2fc6a60c4e77..cfb24593147c 100644
--- a/drivers/infiniband/hw/mlx5/fs.c
+++ b/drivers/infiniband/hw/mlx5/fs.c
@@ -1194,9 +1194,8 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
goto free_ucmd;
}
- if (flow_attr->port > dev->num_ports ||
- (flow_attr->flags &
- ~(IB_FLOW_ATTR_FLAGS_DONT_TRAP | IB_FLOW_ATTR_FLAGS_EGRESS))) {
+ if (flow_attr->flags &
+ ~(IB_FLOW_ATTR_FLAGS_DONT_TRAP | IB_FLOW_ATTR_FLAGS_EGRESS)) {
err = -EINVAL;
goto free_ucmd;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH rdma-rc] RDMA: Verify port when creating flow rule
2021-06-08 5:12 [PATCH rdma-rc] RDMA: Verify port when creating flow rule Leon Romanovsky
@ 2021-06-08 20:09 ` Jason Gunthorpe
2021-06-09 10:55 ` Leon Romanovsky
0 siblings, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2021-06-08 20:09 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Maor Gottlieb, linux-rdma, Mark Bloch
On Tue, Jun 08, 2021 at 08:12:24AM +0300, Leon Romanovsky wrote:
> @@ -3198,6 +3199,13 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
> if (err)
> return err;
>
> + ucontext = ib_uverbs_get_ucontext(attrs);
> + if (IS_ERR(ucontext))
> + return PTR_ERR(ucontext);
ib_uverbs_get_ucontext() should only be used by methods that don't
have a uboject, this one does so it should be using uobj->context
instead
It looks like this can be moved down to after the uobject is allocated
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH rdma-rc] RDMA: Verify port when creating flow rule
2021-06-08 20:09 ` Jason Gunthorpe
@ 2021-06-09 10:55 ` Leon Romanovsky
2021-06-09 11:47 ` Jason Gunthorpe
0 siblings, 1 reply; 5+ messages in thread
From: Leon Romanovsky @ 2021-06-09 10:55 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, Maor Gottlieb, linux-rdma, Mark Bloch
On Tue, Jun 08, 2021 at 05:09:35PM -0300, Jason Gunthorpe wrote:
> On Tue, Jun 08, 2021 at 08:12:24AM +0300, Leon Romanovsky wrote:
> > @@ -3198,6 +3199,13 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
> > if (err)
> > return err;
> >
> > + ucontext = ib_uverbs_get_ucontext(attrs);
> > + if (IS_ERR(ucontext))
> > + return PTR_ERR(ucontext);
>
> ib_uverbs_get_ucontext() should only be used by methods that don't
> have a uboject, this one does so it should be using uobj->context
> instead
Why "should"?
At the end, we will get same ucontext.
>
> It looks like this can be moved down to after the uobject is allocated
The idea is to fail early, before first kmalloc and uobj_alloc, so we won't need
to do any error unwinding.
>
> Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH rdma-rc] RDMA: Verify port when creating flow rule
2021-06-09 10:55 ` Leon Romanovsky
@ 2021-06-09 11:47 ` Jason Gunthorpe
2021-06-09 13:46 ` Leon Romanovsky
0 siblings, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2021-06-09 11:47 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Maor Gottlieb, linux-rdma, Mark Bloch
On Wed, Jun 09, 2021 at 01:55:22PM +0300, Leon Romanovsky wrote:
> On Tue, Jun 08, 2021 at 05:09:35PM -0300, Jason Gunthorpe wrote:
> > On Tue, Jun 08, 2021 at 08:12:24AM +0300, Leon Romanovsky wrote:
> > > @@ -3198,6 +3199,13 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
> > > if (err)
> > > return err;
> > >
> > > + ucontext = ib_uverbs_get_ucontext(attrs);
> > > + if (IS_ERR(ucontext))
> > > + return PTR_ERR(ucontext);
> >
> > ib_uverbs_get_ucontext() should only be used by methods that don't
> > have a uboject, this one does so it should be using uobj->context
> > instead
>
> Why "should"?
> At the end, we will get same ucontext.
The locking methodologies are different, they are not guarenteed to be
exactly the same, but once the uobj is obtained then the related
ucontext is fixed.
> > It looks like this can be moved down to after the uobject is allocated
>
> The idea is to fail early, before first kmalloc and uobj_alloc, so we won't need
> to do any error unwinding.
The error handling is needed anyhow..
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH rdma-rc] RDMA: Verify port when creating flow rule
2021-06-09 11:47 ` Jason Gunthorpe
@ 2021-06-09 13:46 ` Leon Romanovsky
0 siblings, 0 replies; 5+ messages in thread
From: Leon Romanovsky @ 2021-06-09 13:46 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, Maor Gottlieb, linux-rdma, Mark Bloch
On Wed, Jun 09, 2021 at 08:47:20AM -0300, Jason Gunthorpe wrote:
> On Wed, Jun 09, 2021 at 01:55:22PM +0300, Leon Romanovsky wrote:
> > On Tue, Jun 08, 2021 at 05:09:35PM -0300, Jason Gunthorpe wrote:
> > > On Tue, Jun 08, 2021 at 08:12:24AM +0300, Leon Romanovsky wrote:
> > > > @@ -3198,6 +3199,13 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
> > > > if (err)
> > > > return err;
> > > >
> > > > + ucontext = ib_uverbs_get_ucontext(attrs);
> > > > + if (IS_ERR(ucontext))
> > > > + return PTR_ERR(ucontext);
> > >
> > > ib_uverbs_get_ucontext() should only be used by methods that don't
> > > have a uboject, this one does so it should be using uobj->context
> > > instead
> >
> > Why "should"?
> > At the end, we will get same ucontext.
>
> The locking methodologies are different, they are not guarenteed to be
> exactly the same, but once the uobj is obtained then the related
> ucontext is fixed.
>
> > > It looks like this can be moved down to after the uobject is allocated
> >
> > The idea is to fail early, before first kmalloc and uobj_alloc, so we won't need
> > to do any error unwinding.
>
> The error handling is needed anyhow..
Thanks, I'll change.
>
> Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-09 13:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 5:12 [PATCH rdma-rc] RDMA: Verify port when creating flow rule Leon Romanovsky
2021-06-08 20:09 ` Jason Gunthorpe
2021-06-09 10:55 ` Leon Romanovsky
2021-06-09 11:47 ` Jason Gunthorpe
2021-06-09 13:46 ` Leon Romanovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.