Am 26.01.2017 12:23, schrieb Ursula Braun:
>
> 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.
Independend of that ...
would it be possible to make sndbuf_desc->cpu_addr a zero size array
and then simply do a:
sndbuf_desc = kzalloc(sizeof(*sndbuf_desc)+tmp_bufsize, GFP_KERNEL);
(read also: is the whole list of __GPF* really needed ?)
that would make the memhandling more easy
re,
wh
>>
>> Fixes: cd6851f30386 ("smc: remote memory buffers (RMBs)")
>> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>>
>> 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;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
