On Mon, Sep 21, 2020 at 08:07:28PM +0200, Borislav Petkov wrote:

On Tue, Sep 15, 2020 at 02:28:33PM +0300, Jarkko Sakkinen wrote:

quoted

@@ -181,5 +192,12 @@ int __init sgx_drv_init(void)
 		return ret;
 	}
 
+	ret = misc_register(&sgx_dev_provision);
+	if (ret) {
+		pr_err("Creating /dev/sgx/provision failed with %d.\n", ret);
+		misc_deregister(&sgx_dev_enclave);

The comment over misc_deregister() says:

 *      Unregister a miscellaneous device that was previously
 *      successfully registered with misc_register().

but this is not a successful registration here, in the if (ret) case...

'sgx_dev_enclave' is successfully register whenever that happens. Am I
missing something here?

quoted

+ * sgx_ioc_enclave_set_attribute - handler for %SGX_IOC_ENCLAVE_PROVISION
+ * @filep:	open file to /dev/sgx

       ^^^^^^

Can you guess what my comment to that would be...?

There is also another incosistency that I fixed: the first line should
have 'sgx_ioc_enclave_provision'.

quoted

+static long sgx_ioc_enclave_provision(struct sgx_encl *encl,
+					  void __user *arg)

No need for the line break: both function args can fit on the same line.

Fixed this one too, thanks.

...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

/Jarkko