On 09/10/17 14:16, Doug Ledford wrote:
> On Tue, 2017-09-12 at 17:48 +0300, Leon Romanovsky wrote:
>> On Sat, Sep 09, 2017 at 03:56:07PM +0300, Leon Romanovsky wrote:
>>> On Fri, Sep 08, 2017 at 03:37:45PM +0100, Colin King wrote:
>>>> From: Colin Ian King <colin.king@xxxxxxxxxxxxx>
>>>>
>>>> The allocation for elem may fail (especially because we're using
>>>> GFP_ATOMIC) so best to check for a null return. This fixes a
>>>> potential
>>>> null pointer dereference when assigning elem->pool.
>>>>
>>>> Detected by CoverityScan CID#1357507 ("Dereference null return
>>>> value")
>>>>
>>>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>>>> Signed-off-by: Colin Ian King <colin.king@xxxxxxxxxxxxx>
>>>> ---
>>>> drivers/infiniband/sw/rxe/rxe_pool.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c
>>>> b/drivers/infiniband/sw/rxe/rxe_pool.c
>>>> index c1b5f38f31a5..3b4916680018 100644
>>>> --- a/drivers/infiniband/sw/rxe/rxe_pool.c
>>>> +++ b/drivers/infiniband/sw/rxe/rxe_pool.c
>>>> @@ -404,6 +404,8 @@ void *rxe_alloc(struct rxe_pool *pool)
>>>> elem = kmem_cache_zalloc(pool_cache(pool),
>>>> (pool->flags & RXE_POOL_ATOMIC)
>>>> ?
>>>> GFP_ATOMIC : GFP_KERNEL);
>>>> + if (!elem)
>>>> + return NULL;
>>>>
>>>
>>> It is not enough to simply return NULL, you also should release
>>> "pool" too.
>>
>> Colin,
>> do you plan to fix the comment and resend it?
>
> Since Colin is non-responsive in this thread, I went ahead and took his
> patch, but then applied a fixup of my own:
Sorry, I somehow missed that email. Thanks for following this up.
>
> commit a79c0f939da23740c12f43019720055aade89367 (HEAD -> k.o/for-next)
> Author: Doug Ledford <dledford@xxxxxxxxxx>
> Date: Mon Oct 9 09:11:32 2017 -0400
>
> IB/rxe: put the pool on allocation failure
>
> If the allocation of elem fails, it is not sufficient to simply check
> for NULL and return. We need to also put our reference on the pool or
> else we will leave the pool with a permanent ref count and we will never
> be able to free it.
>
> Fixes: 4831ca9e4a8e (IB/rxe: check for allocation failure on elem)
> Suggested-by: Leon Romanovsky <leon@xxxxxxxxxx>
> Signed-off-by: Doug Ledford <dledford@xxxxxxxxxx>
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
> index 3b4916680018..b4a8acc7bb7d 100644
> --- a/drivers/infiniband/sw/rxe/rxe_pool.c
> +++ b/drivers/infiniband/sw/rxe/rxe_pool.c
> @@ -394,23 +394,25 @@ void *rxe_alloc(struct rxe_pool *pool)
>
> kref_get(&pool->rxe->ref_cnt);
>
> - if (atomic_inc_return(&pool->num_elem) > pool->max_elem) {
> - atomic_dec(&pool->num_elem);
> - rxe_dev_put(pool->rxe);
> - rxe_pool_put(pool);
> - return NULL;
> - }
> + if (atomic_inc_return(&pool->num_elem) > pool->max_elem)
> + goto out_put_pool;
>
> elem = kmem_cache_zalloc(pool_cache(pool),
> (pool->flags & RXE_POOL_ATOMIC) ?
> GFP_ATOMIC : GFP_KERNEL);
> if (!elem)
> - return NULL;
> + goto out_put_pool;
>
> elem->pool = pool;
> kref_init(&elem->ref_cnt);
>
> return elem;
> +
> +out_put_pool:
> + atomic_dec(&pool->num_elem);
> + rxe_dev_put(pool->rxe);
> + rxe_pool_put(pool);
> + return NULL;
> }
>
> void rxe_elem_release(struct kref *kref)
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
