[Why] User space might need to inject data into the kernel without allowing it to be read again by any user space. An example of where this is particularly useful is secret keys fetched by user space and injected into the kernel to enable content protection.
[How] Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to create a blob and mark the blob as write only. On reading back the blob, data will be not be copied if it's a write only blob
Signed-off-by: Mark Yacoub markyacoub@chromium.org
--- drivers/gpu/drm/drm_property.c | 3 ++- include/drm/drm_property.h | 2 ++ include/uapi/drm/drm_mode.h | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index dfec479830e4..afedf7109d00 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, if (!blob) return -ENOENT;
- if (out_resp->length == blob->length) { + if (out_resp->length == blob->length && !blob->is_write_only) { if (copy_to_user(u64_to_user_ptr(out_resp->data), blob->data, blob->length)) { @@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev, ret = -EFAULT; goto out_blob; } + blob->is_write_only = out_resp->flags & DRM_MODE_CREATE_BLOB_WRITE_ONLY;
/* Dropping the lock between create_blob and our access here is safe * as only the same file_priv can remove the blob; at this point, it is diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h index 65bc9710a470..700782f021b9 100644 --- a/include/drm/drm_property.h +++ b/include/drm/drm_property.h @@ -205,6 +205,7 @@ struct drm_property { * &drm_mode_config.property_blob_list. * @head_file: entry on the per-file blob list in &drm_file.blobs list. * @length: size of the blob in bytes, invariant over the lifetime of the object + * @is_write_only: user space can't read the blob data. * @data: actual data, embedded at the end of this structure * * Blobs are used to store bigger values than what fits directly into the 64 @@ -219,6 +220,7 @@ struct drm_property_blob { struct list_head head_global; struct list_head head_file; size_t length; + bool is_write_only; void *data; };
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 0a0d56a6158e..de192d3813e9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -1107,6 +1107,9 @@ struct drm_format_modifier { __u64 modifier; };
+#define DRM_MODE_CREATE_BLOB_WRITE_ONLY \ + (1 << 0) /* data of the blob can't be read by user space */ + /** * struct drm_mode_create_blob - Create New blob property * @@ -1120,6 +1123,9 @@ struct drm_mode_create_blob { __u32 length; /** @blob_id: Return: new property ID. */ __u32 blob_id; + /** Flags for special handling. */ + __u32 flags; + __u32 pad; };
/**
friendly ping :)
On Tue, May 10, 2022 at 3:08 PM Mark Yacoub markyacoub@chromium.org wrote:
[Why] User space might need to inject data into the kernel without allowing it to be read again by any user space. An example of where this is particularly useful is secret keys fetched by user space and injected into the kernel to enable content protection.
[How] Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to create a blob and mark the blob as write only. On reading back the blob, data will be not be copied if it's a write only blob
Signed-off-by: Mark Yacoub markyacoub@chromium.org
drivers/gpu/drm/drm_property.c | 3 ++- include/drm/drm_property.h | 2 ++ include/uapi/drm/drm_mode.h | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index dfec479830e4..afedf7109d00 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, if (!blob) return -ENOENT;
if (out_resp->length == blob->length) {
if (out_resp->length == blob->length && !blob->is_write_only) { if (copy_to_user(u64_to_user_ptr(out_resp->data), blob->data, blob->length)) {@@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev, ret = -EFAULT; goto out_blob; }
blob->is_write_only = out_resp->flags & DRM_MODE_CREATE_BLOB_WRITE_ONLY; /* Dropping the lock between create_blob and our access here is safe * as only the same file_priv can remove the blob; at this point, it isdiff --git a/include/drm/drm_property.h b/include/drm/drm_property.h index 65bc9710a470..700782f021b9 100644 --- a/include/drm/drm_property.h +++ b/include/drm/drm_property.h @@ -205,6 +205,7 @@ struct drm_property {
&drm_mode_config.property_blob_list.- @head_file: entry on the per-file blob list in &drm_file.blobs list.
- @length: size of the blob in bytes, invariant over the lifetime of the object
- @is_write_only: user space can't read the blob data.
- @data: actual data, embedded at the end of this structure
- Blobs are used to store bigger values than what fits directly into the 64
@@ -219,6 +220,7 @@ struct drm_property_blob { struct list_head head_global; struct list_head head_file; size_t length;
bool is_write_only; void *data;};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 0a0d56a6158e..de192d3813e9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -1107,6 +1107,9 @@ struct drm_format_modifier { __u64 modifier; };
+#define DRM_MODE_CREATE_BLOB_WRITE_ONLY \
(1 << 0) /* data of the blob can't be read by user space *//**
- struct drm_mode_create_blob - Create New blob property
@@ -1120,6 +1123,9 @@ struct drm_mode_create_blob { __u32 length; /** @blob_id: Return: new property ID. */ __u32 blob_id;
/** Flags for special handling. */__u32 flags;__u32 pad;};
/**
2.36.0.512.ge40c2bad7a-goog
friendly ping :))
On Tue, May 10, 2022 at 3:08 PM Mark Yacoub markyacoub@chromium.org wrote:
[Why] User space might need to inject data into the kernel without allowing it to be read again by any user space. An example of where this is particularly useful is secret keys fetched by user space and injected into the kernel to enable content protection.
[How] Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to create a blob and mark the blob as write only. On reading back the blob, data will be not be copied if it's a write only blob
Signed-off-by: Mark Yacoub markyacoub@chromium.org
drivers/gpu/drm/drm_property.c | 3 ++- include/drm/drm_property.h | 2 ++ include/uapi/drm/drm_mode.h | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index dfec479830e4..afedf7109d00 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, if (!blob) return -ENOENT;
if (out_resp->length == blob->length) {
if (out_resp->length == blob->length && !blob->is_write_only) { if (copy_to_user(u64_to_user_ptr(out_resp->data), blob->data, blob->length)) {@@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev, ret = -EFAULT; goto out_blob; }
blob->is_write_only = out_resp->flags & DRM_MODE_CREATE_BLOB_WRITE_ONLY; /* Dropping the lock between create_blob and our access here is safe * as only the same file_priv can remove the blob; at this point, it isdiff --git a/include/drm/drm_property.h b/include/drm/drm_property.h index 65bc9710a470..700782f021b9 100644 --- a/include/drm/drm_property.h +++ b/include/drm/drm_property.h @@ -205,6 +205,7 @@ struct drm_property {
&drm_mode_config.property_blob_list.- @head_file: entry on the per-file blob list in &drm_file.blobs list.
- @length: size of the blob in bytes, invariant over the lifetime of the object
- @is_write_only: user space can't read the blob data.
- @data: actual data, embedded at the end of this structure
- Blobs are used to store bigger values than what fits directly into the 64
@@ -219,6 +220,7 @@ struct drm_property_blob { struct list_head head_global; struct list_head head_file; size_t length;
bool is_write_only; void *data;};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 0a0d56a6158e..de192d3813e9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -1107,6 +1107,9 @@ struct drm_format_modifier { __u64 modifier; };
+#define DRM_MODE_CREATE_BLOB_WRITE_ONLY \
(1 << 0) /* data of the blob can't be read by user space *//**
- struct drm_mode_create_blob - Create New blob property
@@ -1120,6 +1123,9 @@ struct drm_mode_create_blob { __u32 length; /** @blob_id: Return: new property ID. */ __u32 blob_id;
/** Flags for special handling. */__u32 flags;__u32 pad;};
/**
2.36.0.512.ge40c2bad7a-goog
On Tue, 10 May 2022, Mark Yacoub markyacoub@chromium.org wrote:
[Why] User space might need to inject data into the kernel without allowing it to be read again by any user space. An example of where this is particularly useful is secret keys fetched by user space and injected into the kernel to enable content protection.
I think we're going to need more than an example in the commit message. See Documentation/gpu/drm-uapi.rst.
BR, Jani.
[How] Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to create a blob and mark the blob as write only. On reading back the blob, data will be not be copied if it's a write only blob
Signed-off-by: Mark Yacoub markyacoub@chromium.org
drivers/gpu/drm/drm_property.c | 3 ++- include/drm/drm_property.h | 2 ++ include/uapi/drm/drm_mode.h | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index dfec479830e4..afedf7109d00 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, if (!blob) return -ENOENT;
- if (out_resp->length == blob->length) {
- if (out_resp->length == blob->length && !blob->is_write_only) { if (copy_to_user(u64_to_user_ptr(out_resp->data), blob->data, blob->length)) {
@@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev, ret = -EFAULT; goto out_blob; }
blob->is_write_only = out_resp->flags & DRM_MODE_CREATE_BLOB_WRITE_ONLY;
/* Dropping the lock between create_blob and our access here is safe
- as only the same file_priv can remove the blob; at this point, it is
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h index 65bc9710a470..700782f021b9 100644 --- a/include/drm/drm_property.h +++ b/include/drm/drm_property.h @@ -205,6 +205,7 @@ struct drm_property {
- &drm_mode_config.property_blob_list.
- @head_file: entry on the per-file blob list in &drm_file.blobs list.
- @length: size of the blob in bytes, invariant over the lifetime of the object
- @is_write_only: user space can't read the blob data.
- @data: actual data, embedded at the end of this structure
- Blobs are used to store bigger values than what fits directly into the 64
@@ -219,6 +220,7 @@ struct drm_property_blob { struct list_head head_global; struct list_head head_file; size_t length;
- bool is_write_only; void *data;
};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 0a0d56a6158e..de192d3813e9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -1107,6 +1107,9 @@ struct drm_format_modifier { __u64 modifier; };
+#define DRM_MODE_CREATE_BLOB_WRITE_ONLY \
- (1 << 0) /* data of the blob can't be read by user space */
/**
- struct drm_mode_create_blob - Create New blob property
@@ -1120,6 +1123,9 @@ struct drm_mode_create_blob { __u32 length; /** @blob_id: Return: new property ID. */ __u32 blob_id;
- /** Flags for special handling. */
- __u32 flags;
- __u32 pad;
};
/**
Hi Jani, thanks for your review. I got all the user space implementation ready to see it in context.
libdrm patch to wrap this functionality: https://www.spinics.net/lists/dri-devel/msg347318.html
Chromium user space implementation making direct use of the new prop flag: crrev.com/c/3655850 The first call made to such functionality is in https://chromium-review.googlesource.com/c/chromium/src/+/3655850/2/ui/displ... where the call stack flows to the libdrm wrapper call at https://chromium-review.googlesource.com/c/chromium/src/+/3655850/2/ui/ozone...
I also wrote an IGT test to verify the intended behavior: https://patchwork.freedesktop.org/patch/487331/?series=104373&rev=1
Let me know if I would need to update the commit message with any of the aforementioned context.
Thanks! -Mark Yacoub
On Tue, May 17, 2022 at 3:53 AM Jani Nikula jani.nikula@linux.intel.com wrote:
On Tue, 10 May 2022, Mark Yacoub markyacoub@chromium.org wrote:
[Why] User space might need to inject data into the kernel without allowing it to be read again by any user space. An example of where this is particularly useful is secret keys fetched by user space and injected into the kernel to enable content protection.
I think we're going to need more than an example in the commit message. See Documentation/gpu/drm-uapi.rst.
BR, Jani.
[How] Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to create a blob and mark the blob as write only. On reading back the blob, data will be not be copied if it's a write only blob
Signed-off-by: Mark Yacoub markyacoub@chromium.org
drivers/gpu/drm/drm_property.c | 3 ++- include/drm/drm_property.h | 2 ++ include/uapi/drm/drm_mode.h | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c index dfec479830e4..afedf7109d00 100644 --- a/drivers/gpu/drm/drm_property.c +++ b/drivers/gpu/drm/drm_property.c @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev, if (!blob) return -ENOENT;
if (out_resp->length == blob->length) {
if (out_resp->length == blob->length && !blob->is_write_only) { if (copy_to_user(u64_to_user_ptr(out_resp->data), blob->data, blob->length)) {@@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev, ret = -EFAULT; goto out_blob; }
blob->is_write_only = out_resp->flags & DRM_MODE_CREATE_BLOB_WRITE_ONLY; /* Dropping the lock between create_blob and our access here is safe * as only the same file_priv can remove the blob; at this point, it isdiff --git a/include/drm/drm_property.h b/include/drm/drm_property.h index 65bc9710a470..700782f021b9 100644 --- a/include/drm/drm_property.h +++ b/include/drm/drm_property.h @@ -205,6 +205,7 @@ struct drm_property {
- &drm_mode_config.property_blob_list.
- @head_file: entry on the per-file blob list in &drm_file.blobs list.
- @length: size of the blob in bytes, invariant over the lifetime of the object
- @is_write_only: user space can't read the blob data.
- @data: actual data, embedded at the end of this structure
- Blobs are used to store bigger values than what fits directly into the 64
@@ -219,6 +220,7 @@ struct drm_property_blob { struct list_head head_global; struct list_head head_file; size_t length;
bool is_write_only; void *data;};
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 0a0d56a6158e..de192d3813e9 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -1107,6 +1107,9 @@ struct drm_format_modifier { __u64 modifier; };
+#define DRM_MODE_CREATE_BLOB_WRITE_ONLY \
(1 << 0) /* data of the blob can't be read by user space *//**
- struct drm_mode_create_blob - Create New blob property
@@ -1120,6 +1123,9 @@ struct drm_mode_create_blob { __u32 length; /** @blob_id: Return: new property ID. */ __u32 blob_id;
/** Flags for special handling. */__u32 flags;__u32 pad;};
/**
-- Jani Nikula, Intel Open Source Graphics Center
dri-devel@lists.freedesktop.org
-
Jani Nikula -
Mark Yacoub