On Mon, Jul 31, 2017 at 03:17:24PM +0000, Bart Van Assche wrote:
> On Mon, 2017-07-31 at 10:09 +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > The "lg" variable is declared as int so in all places where
> > this variable is used as a shift operand, the output will be
> > int too.
> >
> > This produces the following smatch warning:
> > drivers/infiniband/hw/mthca/mthca_cmd.c:701 mthca_map_cmd() warn:
> > should '1 << lg' be a 64 bit type?
> >
> > Simple declaration of "1" to be "1ULL" will fix the issue.
> >
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
> > index 9d83a53c0c67..1052c35f2e75 100644
> > --- a/drivers/infiniband/hw/mthca/mthca_cmd.c
> > +++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
> > @@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,
> > for (i = 0; i < mthca_icm_size(&iter) >> lg; ++i) {
> > if (virt != -1) {
> > pages[nent * 2] = cpu_to_be64(virt);
> > - virt += 1 << lg;
> > + virt += 1ULL << lg;
> > }
> >
> > pages[nent * 2 + 1] =
>
> Hello Leon,
>
> Is my analysis correct that lg can be equal to or larger than 32? If so,
> isn't this patch a bug fix that needs a "Fixes:" tag?
This is the fix to commit introduced in pre-git era and I don't have
adequate Fixes tag. The fact that no one complained about the problem
suggests to me that no one is really important to have it in stable trees.
Tariq and me did the analysis for the same code issue with mlx4 (he is
planning to forward the fix to Dave) and our conclusion that it is not
bug.
The reason to it in line 689:drivers/infiniband/hw/mthca/mthca_cmd.c
689 lg = ffs(mthca_icm_addr(&iter) | mthca_icm_size(&iter)) - 1;
The "lg" for sure will be less than 31, because ffs returns values from 0 to 31 and minus 1.
The situation that ffs will return 0 is unlikely to happen.
Thanks
>
> Thanks,
>
> Bart.
Attachment:
signature.asc
Description: PGP signature
