Re: [PATCH v3 05/15] habanalabs: add command buffer module

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Feb 04, 2019 at 10:32:44PM +0200, Oded Gabbay wrote:
> +int hl_cb_ioctl(struct hl_fpriv *hpriv, void *data)
> +{
> +	union hl_cb_args *args = data;
> +	struct hl_device *hdev = hpriv->hdev;
> +	u64 handle;
> +	int rc;
> +
> +	switch (args->in.op) {
> +	case HL_CB_OP_CREATE:
> +		rc = hl_cb_create(hdev, &hpriv->cb_mgr, args->in.cb_size,
> +					&handle, hpriv->ctx->asid);

so cb_size comes from userspace, ok, you check for the value to be too
small, but not too big.  That means someone can try to allocate too much
memory, possibly crashing things, not good :(


> +		memset(args, 0, sizeof(*args));
> +		args->out.cb_handle = handle;
> +		break;
> +	case HL_CB_OP_DESTROY:
> +		rc = hl_cb_destroy(hdev, &hpriv->cb_mgr,
> +					args->in.cb_handle);
> +		memset(args, 0, sizeof(*args));

Why zero this if it's not copied back to userspace?



> +		break;
> +	default:
> +		rc = -EINVAL;

-ENOTTY is normally the "invalid ioctl value", right?

> +		break;
> +	}
> +
> +	return rc;
> +}

thanks,

greg k-h