All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] x86/sgx: Add SGX_IOC_ENCLAVE_CREATE
@ 2025-03-04 13:05 Dan Carpenter
  2025-03-04 22:07 ` Jarkko Sakkinen
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2025-03-04 13:05 UTC (permalink / raw)
  To: Jarkko Sakkinen; +Cc: linux-sgx

Hello Jarkko Sakkinen,

Commit 888d24911787 ("x86/sgx: Add SGX_IOC_ENCLAVE_CREATE") from Nov
13, 2020 (linux-next), leads to the following Smatch static checker
warning:

	arch/x86/kernel/cpu/sgx/ioctl.c:75 sgx_encl_create()
	warn: potential user controlled sizeof overflow 'secs->size + ((1) << 12)' '0-u64max + 4096'

arch/x86/kernel/cpu/sgx/ioctl.c
    57 static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
    58 {
    59         struct sgx_epc_page *secs_epc;
    60         struct sgx_va_page *va_page;
    61         struct sgx_pageinfo pginfo;
    62         struct sgx_secinfo secinfo;
    63         unsigned long encl_size;
    64         struct file *backing;
    65         long ret;
    66 
    67         va_page = sgx_encl_grow(encl, true);
    68         if (IS_ERR(va_page))
    69                 return PTR_ERR(va_page);
    70         else if (va_page)
    71                 list_add(&va_page->list, &encl->va_pages);
    72         /* else the tail page of the VA page list had free slots. */
    73 
    74         /* The extra page goes to SECS. */
--> 75         encl_size = secs->size + PAGE_SIZE;

secs->size is a u64 that comes from the user in sgx_ioc_enclave_create().
If we add PAGE_SIZE that could be an integer overflow.  Also encl_size is
unsigned long so on 32 bits that's another integer overflow.

    76 
    77         backing = shmem_file_setup("SGX backing", encl_size + (encl_size >> 5),
                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Also this can integer overflow.

    78                                    VM_NORESERVE);
    79         if (IS_ERR(backing)) {
    80                 ret = PTR_ERR(backing);
    81                 goto err_out_shrink;
    82         }
    83 
    84         encl->backing = backing;
    85 
    86         secs_epc = sgx_alloc_epc_page(&encl->secs, true);
    87         if (IS_ERR(secs_epc)) {
    88                 ret = PTR_ERR(secs_epc);
    89                 goto err_out_backing;
    90         }
    91 
    92         encl->secs.epc_page = secs_epc;
    93 
    94         pginfo.addr = 0;
    95         pginfo.contents = (unsigned long)secs;
    96         pginfo.metadata = (unsigned long)&secinfo;
    97         pginfo.secs = 0;
    98         memset(&secinfo, 0, sizeof(secinfo));
    99 
    100         ret = __ecreate((void *)&pginfo, sgx_get_epc_virt_addr(secs_epc));
    101         if (ret) {
    102                 ret = -EIO;
    103                 goto err_out;
    104         }
    105 
    106         if (secs->attributes & SGX_ATTR_DEBUG)
    107                 set_bit(SGX_ENCL_DEBUG, &encl->flags);
    108 
    109         encl->secs.encl = encl;
    110         encl->secs.type = SGX_PAGE_TYPE_SECS;
    111         encl->base = secs->base;
    112         encl->size = secs->size;
                ^^^^^^^^^^^^^^^^^^^^^^^
Then we re-use secs->size here.  I haven't followed this code through to
see if anything bad happens, but it makes me itch just looking at it.  :P

    113         encl->attributes = secs->attributes;
    114         encl->attributes_mask = SGX_ATTR_UNPRIV_MASK;
    115 
    116         /* Set only after completion, as encl->lock has not been taken. */
    117         set_bit(SGX_ENCL_CREATED, &encl->flags);
    118 
    119         return 0;
    120 
    121 err_out:
    122         sgx_encl_free_epc_page(encl->secs.epc_page);
    123         encl->secs.epc_page = NULL;
    124 
    125 err_out_backing:
    126         fput(encl->backing);
    127         encl->backing = NULL;
    128 
    129 err_out_shrink:
    130         sgx_encl_shrink(encl, va_page);
    131 
    132         return ret;
    133 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-03-06 18:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-04 13:05 [bug report] x86/sgx: Add SGX_IOC_ENCLAVE_CREATE Dan Carpenter
2025-03-04 22:07 ` Jarkko Sakkinen
2025-03-05  7:44   ` Dan Carpenter
2025-03-06 18:28     ` Jarkko Sakkinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.