[bug report] KVM: SVM: Add KVM_SEND_UPDATE_DATA command

[bug report] KVM: SVM: Add KVM_SEND_UPDATE_DATA command

[Dan Carpenter]

Hello Brijesh Singh,

The patch d3d1af85e2c7: "KVM: SVM: Add KVM_SEND_UPDATE_DATA command"
from Apr 15, 2021, leads to the following static checker warning:

arch/x86/kvm/svm/sev.c:1268 sev_send_update_data() warn: 'guest_page' is an error pointer or valid
arch/x86/kvm/svm/sev.c:1316 sev_send_update_data() warn: maybe return -EFAULT instead of the bytes remaining?
arch/x86/kvm/svm/sev.c:1462 sev_receive_update_data() warn: 'guest_page' is an error pointer or valid

arch/x86/kvm/svm/sev.c
  1261          offset = params.guest_uaddr & (PAGE_SIZE - 1);
  1262          if ((params.guest_len + offset > PAGE_SIZE))
  1263                  return -EINVAL;
  1264  
  1265          /* Pin guest memory */
  1266          guest_page = sev_pin_memory(kvm, params.guest_uaddr & PAGE_MASK,
  1267                                      PAGE_SIZE, &n, 0);
  1268          if (!guest_page)

The sev_pin_memory() function returns error pointers, not NULL.

  1269                  return -EFAULT;
  1270  
  1271          /* allocate memory for header and transport buffer */
  1272          ret = -ENOMEM;
  1273          hdr = kmalloc(params.hdr_len, GFP_KERNEL_ACCOUNT);
  1274          if (!hdr)
  1275                  goto e_unpin;
  1276  
  1277          trans_data = kmalloc(params.trans_len, GFP_KERNEL_ACCOUNT);
  1278          if (!trans_data)
  1279                  goto e_free_hdr;
  1280  
  1281          memset(&data, 0, sizeof(data));
  1282          data.hdr_address = __psp_pa(hdr);
  1283          data.hdr_len = params.hdr_len;
  1284          data.trans_address = __psp_pa(trans_data);
  1285          data.trans_len = params.trans_len;
  1286  
  1287          /* The SEND_UPDATE_DATA command requires C-bit to be always set. */
  1288          data.guest_address = (page_to_pfn(guest_page[0]) << PAGE_SHIFT) + offset;
  1289          data.guest_address |= sev_me_mask;
  1290          data.guest_len = params.guest_len;
  1291          data.handle = sev->handle;
  1292  
  1293          ret = sev_issue_cmd(kvm, SEV_CMD_SEND_UPDATE_DATA, &data, &argp->error);
  1294  
  1295          if (ret)
  1296                  goto e_free_trans_data;
  1297  
  1298          /* copy transport buffer to user space */
  1299          if (copy_to_user((void __user *)(uintptr_t)params.trans_uaddr,
  1300                           trans_data, params.trans_len)) {
  1301                  ret = -EFAULT;
  1302                  goto e_free_trans_data;
  1303          }
  1304  
  1305          /* Copy packet header to userspace. */
  1306          ret = copy_to_user((void __user *)(uintptr_t)params.hdr_uaddr, hdr,
  1307                                  params.hdr_len);

This should be:
		 if (copy_to_user(...))
			ret = -EFAULT;

  1308  
  1309  e_free_trans_data:
  1310          kfree(trans_data);
  1311  e_free_hdr:
  1312          kfree(hdr);
  1313  e_unpin:
  1314          sev_unpin_memory(kvm, guest_page, n);
  1315  
  1316          return ret;
  1317  }

[ snip ]

  1456          data.trans_len = params.trans_len;
  1457  
  1458          /* Pin guest memory */
  1459          ret = -EFAULT;
  1460          guest_page = sev_pin_memory(kvm, params.guest_uaddr & PAGE_MASK,
  1461                                      PAGE_SIZE, &n, 0);
  1462          if (!guest_page)

IS_ERR(guest_page) here as well.

  1463                  goto e_free_trans;
  1464  
  1465          /* The RECEIVE_UPDATE_DATA command requires C-bit to be always set. */
  1466          data.guest_address = (page_to_pfn(guest_page[0]) << PAGE_SHIFT) + offset;
  1467          data.guest_address |= sev_me_mask;
  1468          data.guest_len = params.guest_len;
  1469          data.handle = sev->handle;
  1470  

regards,
dan carpenter

Re: [bug report] KVM: SVM: Add KVM_SEND_UPDATE_DATA command

[Sean Christopherson]

On Thu, Apr 29, 2021, Dan Carpenter wrote:
> Hello Brijesh Singh,
> 
> The patch d3d1af85e2c7: "KVM: SVM: Add KVM_SEND_UPDATE_DATA command"
> from Apr 15, 2021, leads to the following static checker warning:
> 
> arch/x86/kvm/svm/sev.c:1268 sev_send_update_data() warn: 'guest_page' is an error pointer or valid
> arch/x86/kvm/svm/sev.c:1316 sev_send_update_data() warn: maybe return -EFAULT instead of the bytes remaining?
> arch/x86/kvm/svm/sev.c:1462 sev_receive_update_data() warn: 'guest_page' is an error pointer or valid

Thanks for the report.  Is the static checker you're using publicly available?
Catching these bugs via a checker is super cool!

Re: [bug report] KVM: SVM: Add KVM_SEND_UPDATE_DATA command

[Dan Carpenter]

On Thu, May 06, 2021 at 06:09:54PM +0000, Sean Christopherson wrote:
> On Thu, Apr 29, 2021, Dan Carpenter wrote:
> > Hello Brijesh Singh,
> > 
> > The patch d3d1af85e2c7: "KVM: SVM: Add KVM_SEND_UPDATE_DATA command"
> > from Apr 15, 2021, leads to the following static checker warning:
> > 
> > arch/x86/kvm/svm/sev.c:1268 sev_send_update_data() warn: 'guest_page' is an error pointer or valid
> > arch/x86/kvm/svm/sev.c:1316 sev_send_update_data() warn: maybe return -EFAULT instead of the bytes remaining?
> > arch/x86/kvm/svm/sev.c:1462 sev_receive_update_data() warn: 'guest_page' is an error pointer or valid
> 
> Thanks for the report.  Is the static checker you're using publicly available?
> Catching these bugs via a checker is super cool!

This is a Smatch check, but I'm glad you asked about this because it
turns out I never committed the "is an error pointer or valid" check.
I'll do that now and push it later today.

regards,
dan carpenter