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 07:11:47AM +0000, liweihang wrote:
> On 2021/3/22 13:47, Leon Romanovsky wrote:
> > 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
> > 
> 
> Hi Leon,
> 
> By the way, we made this change because we noticed that ib_event_msg() and
> ib_wc_status_msg() that tries to access an array performs input check in the
> same file. Is there anything different between these kernel APIs? Or there is
> some other reasons?

The main difference between them is the execution flow.
 * ib_modify_qp_is_ok() is called from the drivers, after verbs layer
   sanitized everything already and at this stage we are pretty safe.
 * ib_event_msg()/ib_wc_status_ms() are used by ULPs and maybe they can
   send invalid event. I personally don't know if it is possible or not.

Thanks

> 
> Thanks,
> Weihang
>