On Wed, Jun 12, 2024 at 12:17:19PM +0200, Paolo Bonzini wrote:
> Thanks for the report!
>
> > 2134 /* Don't allow userspace to allocate memory for more than 1 SNP context. */
> > 2135 if (sev->snp_context)
> > 2136 return -EINVAL;
> > 2137 2138 sev->snp_context = snp_context_create(kvm,
> > argp);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > What this static checker warning is about is that "argp->sev_fd" points
> > to a file and we create some context here and send a
> > SEV_CMD_SNP_GCTX_CREATE command using that file.
>
> ...
>
> > 2156 start.gctx_paddr = __psp_pa(sev->snp_context);
> > 2157 start.policy = params.policy;
> > 2158 memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));
> > --> 2159 rc = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_LAUNCH_START, &start, &argp->error);
> > ^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^
> > The user controls which file the ->sev_fd points to so now we're doing
> > SEV_CMD_SNP_LAUNCH_START command but the file could be different from
> > what we expected. Does this matter? I don't know KVM well enough to
> > say. It doesn't seem very safe, but it might be fine.
>
> It is safe, all file descriptors for /dev/sev are basically equivalent,
> as they have no file-specific data.
>
> __sev_issue_cmd ends up here:
>
> int sev_issue_cmd_external_user(struct file *filep, unsigned int cmd,
> void *data, int *error)
> {
> if (!filep || filep->f_op != &sev_fops)
> return -EBADF;
>
> return sev_do_cmd(cmd, data, error);
> }
> EXPORT_SYMBOL_GPL(sev_issue_cmd_external_user);
>
> and you can see that the filep argument is only used to check that
> the file has the right file_operations.
Ah. That works. Thanks!
regards,
dan carpenter
