[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xen/privcmd: fix static checker warning
> -----Original Message----- > From: Andrew Cooper > Sent: 07 June 2018 11:28 > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx > Cc: Juergen Gross <jgross@xxxxxxxx>; Boris Ostrovsky > <boris.ostrovsky@xxxxxxxxxx>; Dan Carpenter > <dan.carpenter@xxxxxxxxxx> > Subject: Re: [Xen-devel] [PATCH] xen/privcmd: fix static checker warning > > On 07/06/18 11:21, Paul Durrant wrote: > > Commit 3ad0876554ca ("xen/privcmd: add > IOCTL_PRIVCMD_MMAP_RESOURCE") > > introduced a static checker warning: > > > > drivers/xen/privcmd.c:827 privcmd_ioctl_mmap_resource() > > warn: passing casted pointer 'pfns' to 'xen_remap_domain_mfn_array()' > > 64 vs 32. > > > > caused by this cast: > > > > 827 num = xen_remap_domain_mfn_array(vma, > > 828 kdata.addr & PAGE_MASK, > > 829 pfns, kdata.num, (int *)pfns, > > ^^^^^^^^^^^ > > > > The reason for the cast is that xen_remap_domain_mfn_array() requires > an > > array of ints to store error codes. It is actually safe to re-use the > > pfns array for this purpose but it does look odd (as well as leading to > > the warning). It would also be easy for a future implementation change > > to make this re-use unsafe so this patch modifies privcmd to use a > > separately allocated array for error codes. > > > > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx> > > It may be safe to reuse pfns[] as the storage space for the errs array, > but code is incorrect when sizeof(pfn) != sizeof(int). In such a case, > you skip over every other err, and second half of pfns[] is junk from > the point of view of the errs loop. > Yep, that is indeed what happens without this patch. Paul > ~Andrew > > > --- > > Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> > > Cc: Juergen Gross <jgross@xxxxxxxx> > > Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > --- > > drivers/xen/privcmd.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c > > index 8ae0349..8507c13 100644 > > --- a/drivers/xen/privcmd.c > > +++ b/drivers/xen/privcmd.c > > @@ -822,11 +822,18 @@ static long privcmd_ioctl_mmap_resource(struct > file *file, void __user *udata) > > unsigned int domid = > > (xdata.flags & XENMEM_rsrc_acq_caller_owned) ? > > DOMID_SELF : kdata.dom; > > + int *errs; > > int num; > > > > + errs = kcalloc(kdata.num, sizeof(*errs), GFP_KERNEL); > > + if (!errs) { > > + rc = -ENOMEM; > > + goto out; > > + } > > + > > num = xen_remap_domain_mfn_array(vma, > > kdata.addr & PAGE_MASK, > > - pfns, kdata.num, (int *)pfns, > > + pfns, kdata.num, errs, > > vma->vm_page_prot, > > domid, > > vma->vm_private_data); > > @@ -836,12 +843,14 @@ static long privcmd_ioctl_mmap_resource(struct > file *file, void __user *udata) > > unsigned int i; > > > > for (i = 0; i < num; i++) { > > - rc = pfns[i]; > > + rc = errs[i]; > > if (rc < 0) > > break; > > } > > } else > > rc = 0; > > + > > + kfree(errs); > > } > > > > out: _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |