Hello,
Our static analysis tool reports a possible buffer underflow in
qib_iba6120.c in Linux 5.14.0-rc3:
The variable ctxt is checked in:
2110: if (ctxt < 0)
This indicates that ctxt can be negative.
If so, possible buffer underflows will occur:
2120: qib_write_kreg_ctxt(dd, kr_rcvhdrtailaddr, ctxt,
dd->rcd[ctxt]->rcvhdrqtailaddr_phys);
2122: qib_write_kreg_ctxt(dd, kr_rcvhdraddr, ctxt,
dd->rcd[ctxt]->rcvhdrq_phys);
However, I am not sure whether ctxt < 0 and op & QIB_RCVCTRL_CTXT_ENB
can be true at the same time.
Any feedback would be appreciated, thanks!
Reported-by: TOTE Robot <[email protected]>
Best wishes,
Tuo Li
> Subject: [BUG] RDMA/hw/qib/qib_iba6120: possible buffer underflow in
> rcvctrl_6120_mod()
>
> Hello,
>
> Our static analysis tool reports a possible buffer underflow in qib_iba6120.c in
> Linux 5.14.0-rc3:
>
> The variable ctxt is checked in:
> 2110: if (ctxt < 0)
>
> This indicates that ctxt can be negative.
> If so, possible buffer underflows will occur:
> 2120: qib_write_kreg_ctxt(dd, kr_rcvhdrtailaddr, ctxt,
> dd->rcd[ctxt]->rcvhdrqtailaddr_phys);
> 2122: qib_write_kreg_ctxt(dd, kr_rcvhdraddr, ctxt,
> dd->rcd[ctxt]->rcvhdrq_phys);
>
> However, I am not sure whether ctxt < 0 and op & QIB_RCVCTRL_CTXT_ENB
> can be true at the same time.
>
> Any feedback would be appreciated, thanks!
>
Look at the assignment to f_rcvctrl and the calls using that variable:
5 qib_iba6120.c qib_init_iba6120_funcs 3463 dd->f_rcvctrl = rcvctrl_6120_mod;
6 qib_iba7220.c qib_init_iba7220_funcs 4508 dd->f_rcvctrl = rcvctrl_7220_mod;
7 qib_iba7322.c qib_init_iba7322_funcs 7198 dd->f_rcvctrl = rcvctrl_7322_mod;
All these functions have the same "issue". The -1 parameter implies all contexts and the -1 ctxt happens from init_after_reset() and qib_shutdown_device() and by code inspection, they only or in operations that lack QIB_RCVCTRL_CTXT_ENB, thus avoiding the code path that writes a per context CSR using the ctxt as an index.
I don't think this is an issue.
Mike
> Subject: RE: [BUG] RDMA/hw/qib/qib_iba6120: possible buffer underflow in
> rcvctrl_6120_mod()
>
> > Subject: [BUG] RDMA/hw/qib/qib_iba6120: possible buffer underflow in
> > rcvctrl_6120_mod()
> >
> > Hello,
> >
> > Our static analysis tool reports a possible buffer underflow in
> > qib_iba6120.c in Linux 5.14.0-rc3:
> >
> > The variable ctxt is checked in:
> > 2110: if (ctxt < 0)
> >
> > This indicates that ctxt can be negative.
> > If so, possible buffer underflows will occur:
> > 2120: qib_write_kreg_ctxt(dd, kr_rcvhdrtailaddr, ctxt,
> > dd->rcd[ctxt]->rcvhdrqtailaddr_phys);
> > 2122: qib_write_kreg_ctxt(dd, kr_rcvhdraddr, ctxt,
> > dd->rcd[ctxt]->rcvhdrq_phys);
> >
> > However, I am not sure whether ctxt < 0 and op &
> QIB_RCVCTRL_CTXT_ENB
> > can be true at the same time.
> >
> > Any feedback would be appreciated, thanks!
> >
>
> Look at the assignment to f_rcvctrl and the calls using that variable:
> 5 qib_iba6120.c qib_init_iba6120_funcs 3463 dd->f_rcvctrl =
> rcvctrl_6120_mod;
> 6 qib_iba7220.c qib_init_iba7220_funcs 4508 dd->f_rcvctrl =
> rcvctrl_7220_mod;
> 7 qib_iba7322.c qib_init_iba7322_funcs 7198 dd->f_rcvctrl =
> rcvctrl_7322_mod;
>
> All these functions have the same "issue". The -1 parameter implies all
> contexts and the -1 ctxt happens from init_after_reset() and
> qib_shutdown_device() and by code inspection, they only or in operations
> that lack QIB_RCVCTRL_CTXT_ENB, thus avoiding the code path that writes a
> per context CSR using the ctxt as an index.
>
> I don't think this is an issue.
>
Second thoughs.
See sendctrl_7322_mod().
There is an easy fix as shown in the 7322 version of the f_rcvctrl:
if (ctxt < 0) {
mask = (1ULL << dd->ctxtcnt) - 1;
rcd = NULL;
} else {
mask = (1ULL << ctxt);
rcd = dd->rcd[ctxt];
}
if ((op & QIB_RCVCTRL_CTXT_ENB) && rcd) {
The assignment to the rcd pointer ensures the code can never be reached.
This technique could be ported to the other two routines.
Mike