* [patch -next] qed: potential overflow in qed_cxt_src_t2_alloc()
@ 2016-06-07 12:04 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2016-06-07 12:04 UTC (permalink / raw)
To: Yuval Mintz; +Cc: Ariel Elior, everest-linux-l2, netdev, kernel-janitors
In the current code "ent_per_page" could be more than "conn_num" making
"conn_num" negative after the subtraction. In the next iteration
through the loop then the negative is treated as a very high positive
meaning we don't put a limit on "ent_num". It could lead to memory
corruption.
Fixes: dbb799c39717 ('qed: Initialize hardware for new protocols')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index d85b7ba..1c35f37 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -850,7 +850,7 @@ static int qed_cxt_src_t2_alloc(struct qed_hwfn *p_hwfn)
val = 0;
entries[j].next = cpu_to_be64(val);
- conn_num -= ent_per_page;
+ conn_num -= ent_num;
}
return 0;
^ permalink raw reply related [flat|nested] 6+ messages in thread* [patch -next] qed: potential overflow in qed_cxt_src_t2_alloc()
@ 2016-06-07 12:04 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2016-06-07 12:04 UTC (permalink / raw)
To: Yuval Mintz; +Cc: Ariel Elior, everest-linux-l2, netdev, kernel-janitors
In the current code "ent_per_page" could be more than "conn_num" making
"conn_num" negative after the subtraction. In the next iteration
through the loop then the negative is treated as a very high positive
meaning we don't put a limit on "ent_num". It could lead to memory
corruption.
Fixes: dbb799c39717 ('qed: Initialize hardware for new protocols')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index d85b7ba..1c35f37 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -850,7 +850,7 @@ static int qed_cxt_src_t2_alloc(struct qed_hwfn *p_hwfn)
val = 0;
entries[j].next = cpu_to_be64(val);
- conn_num -= ent_per_page;
+ conn_num -= ent_num;
}
return 0;
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: [patch -next] qed: potential overflow in qed_cxt_src_t2_alloc()
2016-06-07 12:04 ` Dan Carpenter
@ 2016-06-07 12:16 ` Yuval Mintz
-1 siblings, 0 replies; 6+ messages in thread
From: Yuval Mintz @ 2016-06-07 12:16 UTC (permalink / raw)
To: Dan Carpenter
Cc: Ariel Elior, Dept-Eng Everest Linux L2, netdev,
kernel-janitors@vger.kernel.org
> In the current code "ent_per_page" could be more than "conn_num" making
> "conn_num" negative after the subtraction. In the next iteration through the
> loop then the negative is treated as a very high positive meaning we don't put a
> limit on "ent_num". It could lead to memory corruption.
>
> Fixes: dbb799c39717 ('qed: Initialize hardware for new protocols')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
I don't think we can actually hit an issue here, since the number of iterations
was also calculated based on conn_num, so this can only happen on the last
iteration.
Regardless, yours is the correct way to go. Thanks, Yuval.
Acked-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [patch -next] qed: potential overflow in qed_cxt_src_t2_alloc()
@ 2016-06-07 12:16 ` Yuval Mintz
0 siblings, 0 replies; 6+ messages in thread
From: Yuval Mintz @ 2016-06-07 12:16 UTC (permalink / raw)
To: Dan Carpenter
Cc: Ariel Elior, Dept-Eng Everest Linux L2, netdev,
kernel-janitors@vger.kernel.org
> In the current code "ent_per_page" could be more than "conn_num" making
> "conn_num" negative after the subtraction. In the next iteration through the
> loop then the negative is treated as a very high positive meaning we don't put a
> limit on "ent_num". It could lead to memory corruption.
>
> Fixes: dbb799c39717 ('qed: Initialize hardware for new protocols')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
I don't think we can actually hit an issue here, since the number of iterations
was also calculated based on conn_num, so this can only happen on the last
iteration.
Regardless, yours is the correct way to go. Thanks, Yuval.
Acked-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch -next] qed: potential overflow in qed_cxt_src_t2_alloc()
2016-06-07 12:04 ` Dan Carpenter
@ 2016-06-08 7:33 ` David Miller
-1 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2016-06-08 7:33 UTC (permalink / raw)
To: dan.carpenter
Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, netdev,
kernel-janitors
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 7 Jun 2016 15:04:16 +0300
> In the current code "ent_per_page" could be more than "conn_num" making
> "conn_num" negative after the subtraction. In the next iteration
> through the loop then the negative is treated as a very high positive
> meaning we don't put a limit on "ent_num". It could lead to memory
> corruption.
>
> Fixes: dbb799c39717 ('qed: Initialize hardware for new protocols')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch -next] qed: potential overflow in qed_cxt_src_t2_alloc()
@ 2016-06-08 7:33 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2016-06-08 7:33 UTC (permalink / raw)
To: dan.carpenter
Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, netdev,
kernel-janitors
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 7 Jun 2016 15:04:16 +0300
> In the current code "ent_per_page" could be more than "conn_num" making
> "conn_num" negative after the subtraction. In the next iteration
> through the loop then the negative is treated as a very high positive
> meaning we don't put a limit on "ent_num". It could lead to memory
> corruption.
>
> Fixes: dbb799c39717 ('qed: Initialize hardware for new protocols')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-08 7:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-07 12:04 [patch -next] qed: potential overflow in qed_cxt_src_t2_alloc() Dan Carpenter
2016-06-07 12:04 ` Dan Carpenter
2016-06-07 12:16 ` Yuval Mintz
2016-06-07 12:16 ` Yuval Mintz
2016-06-08 7:33 ` David Miller
2016-06-08 7:33 ` David Miller
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.