linux-kernel - Re: [RFC PATCH 09/15] drivers/acrn: add passthrough device support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 16 Aug 2019 16:05:46 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Zhao Yakui <yakui.zhao@...el.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        devel@...verdev.osuosl.org, Gao@...osl.org,
        Shiqing <shiqing.gao@...el.com>,
        Jason Chen CJ <jason.cj.chen@...el.com>
Subject: Re: [RFC PATCH 09/15] drivers/acrn: add passthrough device support

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