Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 16 Aug 2019 16:05:46 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Zhao Yakui <yakui.zhao@intel.com>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org,
        devel@driverdev.osuosl.org, Gao@osuosl.org,
        Shiqing <shiqing.gao@intel.com>,
        Jason Chen CJ <jason.cj.chen@intel.com>
Subject: Re: [RFC PATCH 09/15] drivers/acrn: add passthrough device support
Message-ID: <20190816130546.GA3632@kadam>
References: <1565922356-4488-1-git-send-email-yakui.zhao@intel.com>
 <1565922356-4488-10-git-send-email-yakui.zhao@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1565922356-4488-10-git-send-email-yakui.zhao@intel.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 16, 2019 at 10:25:50AM +0800, Zhao Yakui wrote:
> +	case IC_ASSIGN_PTDEV: {
> +		unsigned short bdf;
> +
> +		if (copy_from_user(&bdf, (void *)ioctl_param,

This casting is ugly and you also need a __user tag for Sparse.  Do
something like "void __user *p = ioctl_param;"

> +				   sizeof(unsigned short)))
> +			return -EFAULT;
> +
> +		ret = hcall_assign_ptdev(vm->vmid, bdf);
> +		if (ret < 0) {
> +			pr_err("acrn: failed to assign ptdev!\n");
> +			return -EFAULT;

Preserve the error code.  "return ret;".

> +		}
> +		break;
> +	}
> +	case IC_DEASSIGN_PTDEV: {
> +		unsigned short bdf;
> +
> +		if (copy_from_user(&bdf, (void *)ioctl_param,
> +				   sizeof(unsigned short)))
> +			return -EFAULT;
> +
> +		ret = hcall_deassign_ptdev(vm->vmid, bdf);
> +		if (ret < 0) {
> +			pr_err("acrn: failed to deassign ptdev!\n");
> +			return -EFAULT;
> +		}
> +		break;
> +	}
> +
> +	case IC_SET_PTDEV_INTR_INFO: {
> +		struct ic_ptdev_irq ic_pt_irq;
> +		struct hc_ptdev_irq *hc_pt_irq;
> +
> +		if (copy_from_user(&ic_pt_irq, (void *)ioctl_param,
> +				   sizeof(ic_pt_irq)))
> +			return -EFAULT;
> +
> +		hc_pt_irq = kmalloc(sizeof(*hc_pt_irq), GFP_KERNEL);
> +		if (!hc_pt_irq)
> +			return -ENOMEM;
> +
> +		memcpy(hc_pt_irq, &ic_pt_irq, sizeof(*hc_pt_irq));

Use memdup_user().

> +
> +		ret = hcall_set_ptdev_intr_info(vm->vmid,
> +						virt_to_phys(hc_pt_irq));
> +		kfree(hc_pt_irq);
> +		if (ret < 0) {
> +			pr_err("acrn: failed to set intr info for ptdev!\n");
> +			return -EFAULT;
> +		}
> +
> +		break;
> +	}

regards,
dan carpenter