| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJUqKUpae5cf-GD1LPQrxfHAMb3K+gY5Q4yxiVCafW9_A9gxfw@mail.gmail.com>
Date: Wed, 29 Jun 2022 12:09:23 -0400
From: Mark Yacoub <markyacoub@...omium.org>
To: Jani Nikula <jani.nikula@...ux.intel.com>
Cc: David Airlie <airlied@...ux.ie>, linux-kernel@...r.kernel.org,
seanpaul@...omium.org, dri-devel@...ts.freedesktop.org,
Thomas Zimmermann <tzimmermann@...e.de>, markyacoub@...gle.com
Subject: Re: [PATCH] drm: Create support for Write-Only property blob
Hi Jani, let me know if you need more info or more changes are needed. Thanks!
On Wed, May 25, 2022 at 3:31 PM Mark Yacoub <markyacoub@...omium.org> wrote:
>
> 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/display/manager/content_protection_key_manager.cc#137
> where the call stack flows to the libdrm wrapper call at
> https://chromium-review.googlesource.com/c/chromium/src/+/3655850/2/ui/ozone/platform/drm/gpu/drm_display.cc#203
>
> 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@...ux.intel.com> wrote:
> >
> > On Tue, 10 May 2022, Mark Yacoub <markyacoub@...omium.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@...omium.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;
> > > };
> > >
> > > /**
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
Powered by blists - more mailing lists