[patch] RDMA/cxgb3: stack info leak in iwch_craete_cq()

[patch] RDMA/cxgb3: stack info leak in iwch_craete_cq()

[Dan Carpenter]

The "uresp.reserved" field isn't initialized.  It's at the end, of the
struct here so we don't need to copy it to the user.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index e87f220..b8e26f2 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -226,7 +226,7 @@ static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve
 			mm->len = PAGE_ALIGN(((1UL << uresp.size_log2) + 1) *
 					     sizeof(struct t3_cqe));
 			uresp.memsize = mm->len;
-			resplen = sizeof uresp;
+			resplen = sizeof uresp - sizeof uresp.reserved;
 		}
 		if (ib_copy_to_udata(udata, &uresp, resplen)) {
 			kfree(mm);

Re: [patch] RDMA/cxgb3: stack info leak in iwch_craete_cq()

[Steve Wise]

On 7/25/2013 12:04 PM, Dan Carpenter wrote:
> The "uresp.reserved" field isn't initialized.  It's at the end, of the
> struct here so we don't need to copy it to the user.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> index e87f220..b8e26f2 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
> +++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> @@ -226,7 +226,7 @@ static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve
>   			mm->len = PAGE_ALIGN(((1UL << uresp.size_log2) + 1) *
>   					     sizeof(struct t3_cqe));
>   			uresp.memsize = mm->len;
> -			resplen = sizeof uresp;
> +			resplen = sizeof uresp - sizeof uresp.reserved;
>   		}
>   		if (ib_copy_to_udata(udata, &uresp, resplen)) {
>   			kfree(mm);

How did you find this?

What if, in the future, the iwch_create_cq_resp struct is changed and 
stuff added to the end?  I'm not sure this optimization is worth it?

Re: [patch] RDMA/cxgb3: stack info leak in iwch_craete_cq()

[Dan Carpenter]

On Thu, Jul 25, 2013 at 01:43:28PM -0500, Steve Wise wrote:
> On 7/25/2013 12:04 PM, Dan Carpenter wrote:
> >The "uresp.reserved" field isn't initialized.  It's at the end, of the
> >struct here so we don't need to copy it to the user.
> >
> >Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> >diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> >index e87f220..b8e26f2 100644
> >--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
> >+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
> >@@ -226,7 +226,7 @@ static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve
> >  			mm->len = PAGE_ALIGN(((1UL << uresp.size_log2) + 1) *
> >  					     sizeof(struct t3_cqe));
> >  			uresp.memsize = mm->len;
> >-			resplen = sizeof uresp;
> >+			resplen = sizeof uresp - sizeof uresp.reserved;
> >  		}
> >  		if (ib_copy_to_udata(udata, &uresp, resplen)) {
> >  			kfree(mm);
> 
> How did you find this?

This is some incoming Smatch stuff.

> 
> What if, in the future, the iwch_create_cq_resp struct is changed
> and stuff added to the end?  I'm not sure this optimization is worth
> it?

Yeah.  I'm not sure either.  I wouldn't do it outside infiniband
code because I was copying other code from there.  I can redo this.

regards,
dan carpenter

[patch v2] RDMA/cxgb3: stack info leak in iwch_craete_cq()

[Dan Carpenter]

The "uresp.reserved" field isn't initialized on this path so it could
leak uninitialized stack information to the user.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: different approach to fixing the problem

diff --git a/drivers/infiniband/hw/cxgb3/iwch_provider.c b/drivers/infiniband/hw/cxgb3/iwch_provider.c
index e87f220..d228383 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_provider.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_provider.c
@@ -226,6 +226,7 @@ static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve
 			mm->len = PAGE_ALIGN(((1UL << uresp.size_log2) + 1) *
 					     sizeof(struct t3_cqe));
 			uresp.memsize = mm->len;
+			uresp.reserved = 0;
 			resplen = sizeof uresp;
 		}
 		if (ib_copy_to_udata(udata, &uresp, resplen)) {

Re: [patch v2] RDMA/cxgb3: stack info leak in iwch_craete_cq()

[Steve Wise]

Acked-by: Steve Wise <swise@opengridcomputing.com>