* [patch net-next] smc: some potential use after free bugs
@ 2017-01-26 9:05 Dan Carpenter
2017-01-26 11:23 ` Ursula Braun
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2017-01-26 9:05 UTC (permalink / raw)
To: Ursula Braun; +Cc: David S. Miller, linux-s390, netdev, kernel-janitors
Say we got really unlucky and these failed on the last iteration, then
it could lead to a use after free bug.
Fixes: cd6851f30386 ("smc: remote memory buffers (RMBs)")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 8b1d34378829..941279e1504e 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -535,6 +535,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
/* if send buffer allocation has failed,
* try a smaller one
*/
+ sndbuf_desc = NULL;
continue;
}
rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
@@ -543,6 +544,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
if (rc) {
kfree(sndbuf_desc->cpu_addr);
kfree(sndbuf_desc);
+ sndbuf_desc = NULL;
continue; /* if mapping failed, try smaller one */
}
sndbuf_desc->used = 1;
@@ -599,6 +601,7 @@ int smc_rmb_create(struct smc_sock *smc)
/* if RMB allocation has failed,
* try a smaller one
*/
+ rmb_desc = NULL;
continue;
}
rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
@@ -607,6 +610,7 @@ int smc_rmb_create(struct smc_sock *smc)
if (rc) {
kfree(rmb_desc->cpu_addr);
kfree(rmb_desc);
+ rmb_desc = NULL;
continue; /* if mapping failed, try smaller one */
}
rc = smc_ib_get_memory_region(lgr->lnk[SMC_SINGLE_LINK].roce_pd,
@@ -619,6 +623,7 @@ int smc_rmb_create(struct smc_sock *smc)
DMA_FROM_DEVICE);
kfree(rmb_desc->cpu_addr);
kfree(rmb_desc);
+ rmb_desc = NULL;
continue;
}
rmb_desc->used = 1;
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [patch net-next] smc: some potential use after free bugs
2017-01-26 9:05 [patch net-next] smc: some potential use after free bugs Dan Carpenter
@ 2017-01-26 11:23 ` Ursula Braun
2017-01-26 11:32 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Ursula Braun @ 2017-01-26 11:23 UTC (permalink / raw)
To: Dan Carpenter; +Cc: David S. Miller, linux-s390, netdev, kernel-janitors
On 01/26/2017 10:05 AM, Dan Carpenter wrote:
> Say we got really unlucky and these failed on the last iteration, then
> it could lead to a use after free bug.
thanks for reporting this! I had already a similar patch prepared, but not
yet submitted. It contains all your added lines plus these additional
pre-initializations at definition time:
@@ -509,7 +509,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
struct smc_connection *conn = &smc->conn;
struct smc_link_group *lgr = conn->lgr;
int tmp_bufsize, tmp_bufsize_short;
- struct smc_buf_desc *sndbuf_desc;
+ struct smc_buf_desc *sndbuf_desc = NULL;
int rc;
/* use socket send buffer size (w/o overhead) as start value */
@@ -573,7 +575,7 @@ int smc_rmb_create(struct smc_sock *smc)
struct smc_connection *conn = &smc->conn;
struct smc_link_group *lgr = conn->lgr;
int tmp_bufsize, tmp_bufsize_short;
- struct smc_buf_desc *rmb_desc;
+ struct smc_buf_desc *rmb_desc = NULL;
int rc;
/* use socket recv buffer size (w/o overhead) as start value */
If you do not contradict, I will post my enhanced patch version.
>
> Fixes: cd6851f30386 ("smc: remote memory buffers (RMBs)")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 8b1d34378829..941279e1504e 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -535,6 +535,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
> /* if send buffer allocation has failed,
> * try a smaller one
> */
> + sndbuf_desc = NULL;
> continue;
> }
> rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
> @@ -543,6 +544,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
> if (rc) {
> kfree(sndbuf_desc->cpu_addr);
> kfree(sndbuf_desc);
> + sndbuf_desc = NULL;
> continue; /* if mapping failed, try smaller one */
> }
> sndbuf_desc->used = 1;
> @@ -599,6 +601,7 @@ int smc_rmb_create(struct smc_sock *smc)
> /* if RMB allocation has failed,
> * try a smaller one
> */
> + rmb_desc = NULL;
> continue;
> }
> rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
> @@ -607,6 +610,7 @@ int smc_rmb_create(struct smc_sock *smc)
> if (rc) {
> kfree(rmb_desc->cpu_addr);
> kfree(rmb_desc);
> + rmb_desc = NULL;
> continue; /* if mapping failed, try smaller one */
> }
> rc = smc_ib_get_memory_region(lgr->lnk[SMC_SINGLE_LINK].roce_pd,
> @@ -619,6 +623,7 @@ int smc_rmb_create(struct smc_sock *smc)
> DMA_FROM_DEVICE);
> kfree(rmb_desc->cpu_addr);
> kfree(rmb_desc);
> + rmb_desc = NULL;
> continue;
> }
> rmb_desc->used = 1;
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch net-next] smc: some potential use after free bugs
2017-01-26 11:23 ` Ursula Braun
@ 2017-01-26 11:32 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2017-01-26 11:32 UTC (permalink / raw)
To: Ursula Braun; +Cc: David S. Miller, linux-s390, netdev, kernel-janitors
On Thu, Jan 26, 2017 at 12:23:02PM +0100, Ursula Braun wrote:
>
> On 01/26/2017 10:05 AM, Dan Carpenter wrote:
> > Say we got really unlucky and these failed on the last iteration, then
> > it could lead to a use after free bug.
> thanks for reporting this! I had already a similar patch prepared, but not
> yet submitted. It contains all your added lines plus these additional
> pre-initializations at definition time:
>
> @@ -509,7 +509,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
> struct smc_connection *conn = &smc->conn;
> struct smc_link_group *lgr = conn->lgr;
> int tmp_bufsize, tmp_bufsize_short;
> - struct smc_buf_desc *sndbuf_desc;
> + struct smc_buf_desc *sndbuf_desc = NULL;
> int rc;
>
> /* use socket send buffer size (w/o overhead) as start value */
>
> @@ -573,7 +575,7 @@ int smc_rmb_create(struct smc_sock *smc)
> struct smc_connection *conn = &smc->conn;
> struct smc_link_group *lgr = conn->lgr;
> int tmp_bufsize, tmp_bufsize_short;
> - struct smc_buf_desc *rmb_desc;
> + struct smc_buf_desc *rmb_desc = NULL;
> int rc;
>
> /* use socket recv buffer size (w/o overhead) as start value */
>
> If you do not contradict, I will post my enhanced patch version.
Those are obviously harmless changes...
My static checker knows those aren't required so it doesn't complain
about uninitialized variables. But not all static analysis tools are as
clever. ;)
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-26 11:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-26 9:05 [patch net-next] smc: some potential use after free bugs Dan Carpenter
2017-01-26 11:23 ` Ursula Braun
2017-01-26 11:32 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).