| Message ID | 1523370383-6766-1-git-send-email-shamir.rabinovitch@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Apr 10, 2018 at 10:26:23AM -0400, Shamir Rabinovitch wrote: > Commit 6a21dfc ("RDMA/ucma: Limit possible option size") introduced > a bug. running below command would cause tool hanged task warning: > $ udaddy -c 1000 -C 1000 -S 1024 -t 3 -s <server-ip> > > Below prints explain what happen: > udaddy: set TOS option failed: Invalid argument > > This issue is taken care by commit, 5f3e3b8 ("RDMA/ucma: Correct option > size check using optlen") however this commit did not fix another > issue introduced by commit 6a21dfc where ucma_context ref can leak in > error path. > > Fix it! > > Fixes: 6a21dfc ("6a21dfc RDMA/ucma: Limit possible option size") > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> > --- > drivers/infiniband/core/ucma.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c > index 7432948..53cac78 100644 > --- a/drivers/infiniband/core/ucma.c > +++ b/drivers/infiniband/core/ucma.c > @@ -1320,8 +1320,10 @@ static ssize_t ucma_set_option(struct ucma_file *file, const char __user *inbuf, > if (IS_ERR(ctx)) > return PTR_ERR(ctx); > > - if (unlikely(cmd.optlen > KMALLOC_MAX_SIZE)) > - return -EINVAL; > + if (unlikely(cmd.optlen > KMALLOC_MAX_SIZE)) { > + ret = -EINVAL; > + goto out; > + } > The better way to fix it is to put that chunk of the code above ucma_get_ctx(), but this solution is good enough too. Thanks, Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
On Tue, Apr 10, 2018 at 10:26:23AM -0400, Shamir Rabinovitch wrote: > Commit 6a21dfc ("RDMA/ucma: Limit possible option size") introduced > a bug. running below command would cause tool hanged task warning: > $ udaddy -c 1000 -C 1000 -S 1024 -t 3 -s <server-ip> > > Below prints explain what happen: > udaddy: set TOS option failed: Invalid argument > > This issue is taken care by commit, 5f3e3b8 ("RDMA/ucma: Correct option > size check using optlen") however this commit did not fix another > issue introduced by commit 6a21dfc where ucma_context ref can leak in > error path. > > Fix it! > > Fixes: 6a21dfc ("6a21dfc RDMA/ucma: Limit possible option size") > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> > Reviewed-by: Leon Romanovsky <leonro@mellanox.com> > --- > drivers/infiniband/core/ucma.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) Applied to for-next, but I changed the patch to just move the check before the get and rewrote the commit message. Also please remember to run checkpatch Thanks, Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 10, 2018 at 10:26:23AM -0400, Shamir Rabinovitch wrote: > Commit 6a21dfc ("RDMA/ucma: Limit possible option size") introduced > a bug. running below command would cause tool hanged task warning: > $ udaddy -c 1000 -C 1000 -S 1024 -t 3 -s <server-ip> > > Below prints explain what happen: > udaddy: set TOS option failed: Invalid argument > > This issue is taken care by commit, 5f3e3b8 ("RDMA/ucma: Correct option > size check using optlen") however this commit did not fix another > issue introduced by commit 6a21dfc where ucma_context ref can leak in > error path. > > Fix it! > > Fixes: 6a21dfc ("6a21dfc RDMA/ucma: Limit possible option size") > Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> > --- > drivers/infiniband/core/ucma.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c > index 7432948..53cac78 100644 > --- a/drivers/infiniband/core/ucma.c > +++ b/drivers/infiniband/core/ucma.c > @@ -1320,8 +1320,10 @@ static ssize_t ucma_set_option(struct ucma_file *file, const char __user *inbuf, > if (IS_ERR(ctx)) > return PTR_ERR(ctx); > > - if (unlikely(cmd.optlen > KMALLOC_MAX_SIZE)) > - return -EINVAL; > + if (unlikely(cmd.optlen > KMALLOC_MAX_SIZE)) { > + ret = -EINVAL; > + goto out; > + } > > optval = memdup_user(u64_to_user_ptr(cmd.optval), > cmd.optlen); > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c index 7432948..53cac78 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -1320,8 +1320,10 @@ static ssize_t ucma_set_option(struct ucma_file *file, const char __user *inbuf, if (IS_ERR(ctx)) return PTR_ERR(ctx); - if (unlikely(cmd.optlen > KMALLOC_MAX_SIZE)) - return -EINVAL; + if (unlikely(cmd.optlen > KMALLOC_MAX_SIZE)) { + ret = -EINVAL; + goto out; + } optval = memdup_user(u64_to_user_ptr(cmd.optval), cmd.optlen);
Commit 6a21dfc ("RDMA/ucma: Limit possible option size") introduced a bug. running below command would cause tool hanged task warning: $ udaddy -c 1000 -C 1000 -S 1024 -t 3 -s <server-ip> Below prints explain what happen: udaddy: set TOS option failed: Invalid argument This issue is taken care by commit, 5f3e3b8 ("RDMA/ucma: Correct option size check using optlen") however this commit did not fix another issue introduced by commit 6a21dfc where ucma_context ref can leak in error path. Fix it! Fixes: 6a21dfc ("6a21dfc RDMA/ucma: Limit possible option size") Signed-off-by: Shamir Rabinovitch <shamir.rabinovitch@oracle.com> --- drivers/infiniband/core/ucma.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)