[bug report] virt: Add SEV-SNP guest driver

[bug report] virt: Add SEV-SNP guest driver

[Dan Carpenter]

Hello Brijesh Singh,

The patch fce96cf04430: "virt: Add SEV-SNP guest driver" from Mar 7,
2022, leads to the following Smatch static checker warning:

drivers/virt/coco/sevguest/sevguest.c:298 enc_payload() warn: signedness bug returning '(-63)'
drivers/virt/coco/sevguest/sevguest.c:329 handle_guest_request() error: uninitialized symbol 'err'.
drivers/virt/coco/sevguest/sevguest.c:584 alloc_shared_pages() warn: 'page' is not an error pointer

drivers/virt/coco/sevguest/sevguest.c
   279        static bool enc_payload(struct snp_guest_dev *snp_dev, u64 seqno, int version, u8 type,
                     ^^^^
This should be int.

   280                                void *payload, size_t sz)
   281        {
   282                struct snp_guest_msg *req = snp_dev->request;
   283                struct snp_guest_msg_hdr *hdr = &req->hdr;
   284
   285                memset(req, 0, sizeof(*req));
   286
   287                hdr->algo = SNP_AEAD_AES_256_GCM;
   288                hdr->hdr_version = MSG_HDR_VER;
   289                hdr->hdr_sz = sizeof(*hdr);
   290                hdr->msg_type = type;
   291                hdr->msg_version = version;
   292                hdr->msg_seqno = seqno;
   293                hdr->msg_vmpck = vmpck_id;
   294                hdr->msg_sz = sz;
   295
   296                /* Verify the sequence number is non-zero */
   297                if (!hdr->msg_seqno)
   298                        return -ENOSR;
   299
   300                dev_dbg(snp_dev->dev, "request [seqno %lld type %d version %d sz %d]\n",
   301                        hdr->msg_seqno, hdr->msg_type, hdr->msg_version, hdr->msg_sz);
   302
   303                return __enc_payload(snp_dev, req, payload, sz);
   304        }
   305
   306        static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, int msg_ver,
   307                                        u8 type, void *req_buf, size_t req_sz, void *resp_buf,
   308                                        u32 resp_sz, __u64 *fw_err)
   309        {
   310                unsigned long err;
   311                u64 seqno;
   312                int rc;
   313
   314                /* Get message sequence and verify that its a non-zero */
   315                seqno = snp_get_msg_seqno(snp_dev);
   316                if (!seqno)
   317                        return -EIO;
   318
   319                memset(snp_dev->response, 0, sizeof(struct snp_guest_msg));
   320
   321                /* Encrypt the userspace provided payload */
   322                rc = enc_payload(snp_dev, seqno, msg_ver, type, req_buf, req_sz);
   323                if (rc)
   324                        return rc;
   325
   326                /* Call firmware to process the request */
   327                rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
   328                if (fw_err)
   329                        *fw_err = err;

"err" can be uninitialized.

   330
   331                if (rc)
   332                        return rc;
   333
   334                /*
   335                 * The verify_and_dec_payload() will fail only if the hypervisor is
   336                 * actively modifying the message header or corrupting the encrypted payload.
   337                 * This hints that hypervisor is acting in a bad faith. Disable the VMPCK so that
   338                 * the key cannot be used for any communication. The key is disabled to ensure
   339                 * that AES-GCM does not use the same IV while encrypting the request payload.
   340                 */
   341                rc = verify_and_dec_payload(snp_dev, resp_buf, resp_sz);
   342                if (rc) {
   343                        dev_alert(snp_dev->dev,
   344                                  "Detected unexpected decode failure, disabling the vmpck_id %d\n",
   345                                  vmpck_id);
   346                        snp_disable_vmpck(snp_dev);
   347                        return rc;
   348                }
   349
   350                /* Increment to new message sequence after payload decryption was successful. */
   351                snp_inc_msg_seqno(snp_dev);
   352
   353                return 0;
   354        }

[ snip ]

   577        static void *alloc_shared_pages(size_t sz)
   578        {
   579                unsigned int npages = PAGE_ALIGN(sz) >> PAGE_SHIFT;
   580                struct page *page;
   581                int ret;
   582
   583                page = alloc_pages(GFP_KERNEL_ACCOUNT, get_order(sz));
   584                if (IS_ERR(page))

if (!page)

   585                        return NULL;
   586
   587                ret = set_memory_decrypted((unsigned long)page_address(page), npages);
   588                if (ret) {
   589                        pr_err("failed to mark page shared, ret=%d\n", ret);
   590                        __free_pages(page, get_order(sz));
   591                        return NULL;
   592                }
   593
   594                return page_address(page);
   595        }
   596

regards,
dan carpenter