Skip to contentSkip to conversation

RDMA/cxgb3: stack info leak in iwch_craete_cq()

PATCH Superseded Dan Carpenter 2 participants 5 messages Jul 25, 2013 – Jul 29, 2013 3 messages not in the archive

Subsystems (2)
Kernel janitors InfiniBand
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 RDMA/cxgb3: stack info leak in iwch_craete_cq()

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 <[email protected]>
drivers/infiniband/hw/cxgb3/iwch_provider.c +1 -1
index e87f220..b8e26f2 100644
@@ -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);

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 10 lines · 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 <[email protected]>
drivers/infiniband/hw/cxgb3/iwch_provider.c +0 -0
index e87f220..b8e26f2 100644

drivers/infiniband/hw/cxgb3/iwch_provider.c static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve · line 232

Quoted 8 lines
drivers/infiniband/hw/cxgb3/iwch_provider.c +1 -1
@@ -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?

A message that is not in the archive (sent off-list or not yet received): 51F171D0.7050204-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org

author
#4 lore raw
Quoted 6 lines · Steve Wise
Quoted 5 lines · 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 <[email protected]>
Quoted 15 lines

drivers/infiniband/hw/cxgb3/iwch_provider.c static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve · line 232

Quoted 13 lines
drivers/infiniband/hw/cxgb3/iwch_provider.c +1 -1
index e87f220..b8e26f2 100644
@@ -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.

Quoted 4 lines

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

author
#5 lore raw

PATCH v2 1/1 RDMA/cxgb3: stack info leak in iwch_craete_cq()

In reply to Steve Wise

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 <[email protected]>

---
v2: different approach to fixing the problem

drivers/infiniband/hw/cxgb3/iwch_provider.c +1 -0
index e87f220..d228383 100644
@@ -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)) {

A message that is not in the archive (sent off-list or not yet received): [email protected]