Re: [PATCH for-next] RDMA/core: Check invalid QP state for ib_modify_qp_is_ok()

[Leon Romanovsky <leon@kernel.org>]

On Mon, Mar 22, 2021 at 03:29:09AM +0000, liweihang wrote:
> On 2021/3/20 17:34, Leon Romanovsky wrote:
> > On Fri, Mar 19, 2021 at 05:02:25PM +0800, Weihang Li wrote:
> >> From: Xi Wang <wangxi11@huawei.com>
> >>
> >> Out-of-bounds may occur in 'qp_state_table' when the caller passing wrong
> >> QP state value.
> > 
> > How is it possible? Do you have call stack to support it?
> > 
> > Thanks
> > 
> 
> ib_modify_qp_is_ok() is exported, I think any kernel modules can pass in
> invalid QP state. Should we check it in such case?

No, it is caller responsibility to supply valid input.
In general case, for the kernel code, it can be seen as anti-pattern
if in-kernel API performs input sanity check.

You can add WARN_ON() if you want to catch programmers errors earlier.
However, I'm skeptical if it is really needed here. 

Thanks

> 
> Thanks
> Weihang
> 
> >>
> >> Signed-off-by: Xi Wang <wangxi11@huawei.com>
> >> Signed-off-by: Weihang Li <liweihang@huawei.com>
> >> ---
> >>  drivers/infiniband/core/verbs.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> >> index 28464c5..66ba4e6 100644
> >> --- a/drivers/infiniband/core/verbs.c
> >> +++ b/drivers/infiniband/core/verbs.c
> >> @@ -1613,6 +1613,10 @@ bool ib_modify_qp_is_ok(enum ib_qp_state cur_state, enum ib_qp_state next_state,
> >>  	    cur_state != IB_QPS_SQD && cur_state != IB_QPS_SQE)
> >>  		return false;
> >>  
> >> +	if (cur_state >= ARRAY_SIZE(qp_state_table) ||
> >> +	    next_state >= ARRAY_SIZE(qp_state_table[0]))
> >> +		return false;
> >> +
> >>  	if (!qp_state_table[cur_state][next_state].valid)
> >>  		return false;
> >>  
> >> -- 
> >> 2.8.1
> >>
>