On Wed, Oct 19, 2022 at 09:58:27PM -0700, Kuppuswamy Sathyanarayanan wrote:
+static long tdx_get_report(void __user *argp) +{
- u8 *reportdata, *tdreport;
- struct tdx_report_req req;
- long ret;
- if (copy_from_user(&req, argp, sizeof(req)))
return -EFAULT;- /*
* Per TDX Module 1.0 specification, section titled* "TDG.MR.REPORT", REPORTDATA length is fixed as* TDX_REPORTDATA_LEN, TDREPORT length is fixed as* TDX_REPORT_LEN, and TDREPORT subtype is fixed as 0.*/- if (req.subtype || req.rpd_len != TDX_REPORTDATA_LEN ||
req.tdr_len != TDX_REPORT_LEN) {pr_err("TDX_CMD_GET_REPORT: invalid req: subtype:%u rpd_len:%u tdr_len:%u\n",req.subtype, req.rpd_len, req.tdr_len);
You are allowing userspace to spam the kernel logs, please do not do that.
Also, you have a real device here, use it and call dev_*() instead of pr_*(). Your code should not have any pr_* calls.
return -EINVAL;- }
- if (memchr_inv(req.reserved, 0, sizeof(req.reserved))) {
pr_err("TDX_CMD_GET_REPORT: Non zero value in reserved field\n");return -EINVAL;- }
- reportdata = kmalloc(req.rpd_len, GFP_KERNEL);
- if (!reportdata)
return -ENOMEM;- tdreport = kzalloc(req.tdr_len, GFP_KERNEL);
- if (!tdreport) {
ret = -ENOMEM;goto out;- }
- if (copy_from_user(reportdata, u64_to_user_ptr(req.reportdata),
req.rpd_len)) {ret = -EFAULT;goto out;- }
- /* Generate TDREPORT using "TDG.MR.REPORT" TDCALL */
- ret = tdx_mcall_get_report(reportdata, tdreport, req.subtype);
- if (ret) {
pr_err("TDX_CMD_GET_REPORT: TDCALL failed\n");goto out;- }
- if (copy_to_user(u64_to_user_ptr(req.tdreport), tdreport, req.tdr_len))
ret = -EFAULT;+out:
- kfree(reportdata);
- kfree(tdreport);
- return ret;
+}
+static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)+{
- switch (cmd) {
- case TDX_CMD_GET_REPORT:
return tdx_get_report((void __user *)arg);- default:
return -ENOTTY;- }
+}
+static const struct file_operations tdx_guest_fops = {
- .owner = THIS_MODULE,
- .unlocked_ioctl = tdx_guest_ioctl,
- .llseek = no_llseek,
+};
+static struct miscdevice tdx_misc_dev = {
- .name = KBUILD_MODNAME,
- .minor = MISC_DYNAMIC_MINOR,
- .fops = &tdx_guest_fops,
+};
+static int __init tdx_guest_init(void) +{
- if (!cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
return -ENODEV;- return misc_register(&tdx_misc_dev);
+} +module_init(tdx_guest_init);
+static void __exit tdx_guest_exit(void) +{
- misc_deregister(&tdx_misc_dev);
+} +module_exit(tdx_guest_exit);
+#ifdef MODULE +static const struct x86_cpu_id tdx_guest_ids[] = {
- X86_MATCH_FEATURE(X86_FEATURE_TDX_GUEST, NULL),
- {}
+}; +MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids); +#endif
Why the #ifdef? Should not be needed, right?
+MODULE_AUTHOR("Kuppuswamy Sathyanarayanan sathyanarayanan.kuppuswamy@linux.intel.com"); +MODULE_DESCRIPTION("TDX Guest Driver"); +MODULE_LICENSE("GPL"); diff --git a/include/uapi/linux/tdx-guest.h b/include/uapi/linux/tdx-guest.h new file mode 100644 index 000000000000..fd71774a90ac --- /dev/null +++ b/include/uapi/linux/tdx-guest.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef _UAPI_LINUX_TDX_GUEST_H_ +#define _UAPI_LINUX_TDX_GUEST_H_
No Intel copyright notice? Are you sure you ran this code through internal Intel review properly?
{sigh}
+#include <linux/ioctl.h> +#include <linux/types.h>
+/* Length of the REPORTDATA used in TDG.MR.REPORT TDCALL */ +#define TDX_REPORTDATA_LEN 64
+/* Length of TDREPORT used in TDG.MR.REPORT TDCALL */ +#define TDX_REPORT_LEN 1024
+/**
- struct tdx_report_req - Get TDREPORT using REPORTDATA as input.
- @reportdata: User-defined REPORTDATA to be included into TDREPORT.
Typically it can be some nonce provided by attestation
service, so the generated TDREPORT can be uniquely verified.
- @tdreport: TDREPORT output from TDCALL[TDG.MR.REPORT].
- @rpd_len: Length of the REPORTDATA (fixed as 64 bytes by the TDX Module
specification, but a parameter is added to handle future
extension).
- @tdr_len: Length of the TDREPORT (fixed as 1024 bytes by the TDX Module
specification, but a parameter is added to accommodate future
extension).
- @subtype: Subtype of TDREPORT (fixed as 0 by the TDX Module specification,
but added a parameter to handle future extension).
- @reserved: Reserved entries to handle future requirements. Should be
s/Should/Must/
thanks,
greg k-h