Messages in this thread | | | Subject | Re: [Xen-devel] [PATCH] xen/privcmd: fix static checker warning | From | Juergen Gross <> | Date | Thu, 7 Jun 2018 16:03:05 +0200 |
| |
On 07/06/18 13:23, Paul Durrant wrote: >> -----Original Message----- >> From: Andrew Cooper >> Sent: 07 June 2018 11:28 >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org; >> linux-kernel@vger.kernel.org >> Cc: Juergen Gross <jgross@suse.com>; Boris Ostrovsky >> <boris.ostrovsky@oracle.com>; Dan Carpenter >> <dan.carpenter@oracle.com> >> 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@oracle.com> >>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com> >> >> 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.
Can you please update the commit message accordingly?
Juergen
| |