On Tue, Apr 18, 2017 at 04:48:56PM +0300, Yuval Shaia wrote: > On Sat, Apr 15, 2017 at 06:47:25PM +0300, Leon Romanovsky wrote: > > The mthca driver didn't check supplied pointer to functions > > mthca_cmd_poll() and mthca_cmd_wait(). This caused to the following > > smatch errors: > > > > drivers/infiniband/hw/mthca/mthca_cmd.c:371 mthca_cmd_poll() error: we previously assumed 'out_param' could be null (see line 353) > > drivers/infiniband/hw/mthca/mthca_cmd.c:454 mthca_cmd_wait() error: we previously assumed 'out_param' could be null (see line 432) > > > > In reality all callers of these functions are setting out_is_imm > > flag are providing pointer too. However it is better to check > > again to remove smatch errors to achieve warning free subsystem. > > > > Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx> > > --- > > Based on k.o/for-4.12 branch. > > --- > > drivers/infiniband/hw/mthca/mthca_cmd.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c > > index c7f49bbb0c72..9d83a53c0c67 100644 > > --- a/drivers/infiniband/hw/mthca/mthca_cmd.c > > +++ b/drivers/infiniband/hw/mthca/mthca_cmd.c > > @@ -367,12 +367,16 @@ static int mthca_cmd_poll(struct mthca_dev *dev, > > goto out; > > } > > > > - if (out_is_imm) > > + if (out_is_imm && out_param) { > > *out_param = > > (u64) be32_to_cpu((__force __be32) > > __raw_readl(dev->hcr + HCR_OUT_PARAM_OFFSET)) << 32 | > > (u64) be32_to_cpu((__force __be32) > > __raw_readl(dev->hcr + HCR_OUT_PARAM_OFFSET + 4)); > > + } else if (out_is_imm) { > > + err = -EINVAL; > > + goto out; > > So if this is a good reason to fail the function - how about doing this > validation before triggering HW operation (mthca_cmd_post)? It doesn't matter, it will give the same bytecode. > > > + } > > > > status = be32_to_cpu((__force __be32) __raw_readl(dev->hcr + HCR_STATUS_OFFSET)) >> 24; > > if (status) { > > @@ -450,8 +454,12 @@ static int mthca_cmd_wait(struct mthca_dev *dev, > > err = mthca_status_to_errno(context->status); > > } > > > > - if (out_is_imm) > > + if (out_is_imm && out_param) { > > *out_param = context->out_param; > > + } else if (out_is_imm) { > > + err = -EINVAL; > > Ditto > > > + goto out; > > + } > > > > out: > > spin_lock(&dev->cmd.context_lock); > > -- > > 2.12.2 > > > > -- > > 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
Attachment:
signature.asc
Description: PGP signature