On 8/21/21 8:00 AM, yangx.jy@xxxxxxxxxxx wrote:
> On 2021/8/21 2:44, Bob Pearson wrote:
>> On 8/20/21 6:15 AM, Xiao Yang wrote:
>>> 1) New index member of struct rxe_queue is introduced but not zeroed
>>> so the initial value of index may be random.
>>> 2) Current index is not masked off to index_mask.
>>> In such case, producer_addr() and consumer_addr() will get an invalid
>>> address by the random index and then accessing the invalid address
>>> triggers the following panic:
>>> "BUG: unable to handle page fault for address: ffff9ae2c07a1414"
>>>
>>> Fix the issue by using kzalloc() to zero out index member.
>>>
>>> Fixes: 5bcf5a59c41e ("RDMA/rxe: Protext kernel index from user space")
>>> Signed-off-by: Xiao Yang<yangx.jy@xxxxxxxxxxx>
>>> ---
>>> drivers/infiniband/sw/rxe/rxe_queue.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_queue.c b/drivers/infiniband/sw/rxe/rxe_queue.c
>>> index 85b812586ed4..72d95398e604 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_queue.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_queue.c
>>> @@ -63,7 +63,7 @@ struct rxe_queue *rxe_queue_init(struct rxe_dev *rxe, int *num_elem,
>>> if (*num_elem< 0)
>>> goto err1;
>>>
>>> - q = kmalloc(sizeof(*q), GFP_KERNEL);
>>> + q = kzalloc(sizeof(*q), GFP_KERNEL);
>>> if (!q)
>>> goto err1;
>>>
>>>
>> Thanks for this!! I am happy to take the blame but this has been there from the original 2016 rxe commit. Its a good catch.
> Hi Bob,
>
> The original 2016 rxe commit actually introduced kmalloc() but it
> initialized all members of struct rxe_queue at subsequent steps.
> When the new index member of struct rxe_queue was added, it didn't
> initialized at subsequent steps. So I think the issue was caused by
> your patch.
Yup. My comment was really that if it was an old one I was guilty either way most likely. But this is a good catch.
> I use kzalloc() to fix the issue because I want to avoid the same issue
> when another new member will be added in future.
>
> Best Regards,
> Xiao Yang
>> Reviewed-by: Bob Pearson<rpearsonhpe@xxxxxxxxx>
