Re: [PATCH AUTOSEL 4.14 6/7] drm/nouveau: block a bunch of classes from userspace
From: Lyude Paul
Date: Tue Aug 24 2021 - 13:08:02 EST
This isn't at all intended to be a fix to be backported, so I don't think this
should be included. I don't know about 5/7, but I'll let Benjamin comment on
that one
On Mon, 2021-08-23 at 20:55 -0400, Sasha Levin wrote:
> From: Ben Skeggs <bskeggs@xxxxxxxxxx>
>
> [ Upstream commit 148a8653789c01f159764ffcc3f370008966b42f ]
>
> Long ago, there had been plans for making use of a bunch of these APIs
> from userspace and there's various checks in place to stop misbehaving.
>
> Countless other projects have occurred in the meantime, and the pieces
> didn't finish falling into place for that to happen.
>
> They will (hopefully) in the not-too-distant future, but it won't look
> quite as insane. The super checks are causing problems right now, and
> are going to be removed.
>
> Signed-off-by: Ben Skeggs <bskeggs@xxxxxxxxxx>
> Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
> drivers/gpu/drm/nouveau/include/nvif/cl0080.h | 3 +-
> drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
> drivers/gpu/drm/nouveau/nouveau_usif.c | 57 ++++++++++++++-----
> .../gpu/drm/nouveau/nvkm/engine/device/user.c | 2 +-
> 4 files changed, 48 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
> b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
> index 2740278d226b..61c17acd507c 100644
> --- a/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
> +++ b/drivers/gpu/drm/nouveau/include/nvif/cl0080.h
> @@ -4,7 +4,8 @@
>
> struct nv_device_v0 {
> __u8 version;
> - __u8 pad01[7];
> + __u8 priv;
> + __u8 pad02[6];
> __u64 device; /* device identifier, ~0 for client default */
> };
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index fb6b1d0f7fef..fc54a26598cc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -151,6 +151,7 @@ nouveau_cli_init(struct nouveau_drm *drm, const char
> *sname,
> ret = nvif_device_init(&cli->base.object, 0, NV_DEVICE,
> &(struct nv_device_v0) {
> .device = ~0,
> + .priv = true,
> }, sizeof(struct nv_device_v0),
> &cli->device);
> if (ret) {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_usif.c
> b/drivers/gpu/drm/nouveau/nouveau_usif.c
> index 9dc10b17ad34..5da1f4d223d7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_usif.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_usif.c
> @@ -32,6 +32,9 @@
> #include <nvif/event.h>
> #include <nvif/ioctl.h>
>
> +#include <nvif/class.h>
> +#include <nvif/cl0080.h>
> +
> struct usif_notify_p {
> struct drm_pending_event base;
> struct {
> @@ -261,7 +264,7 @@ usif_object_dtor(struct usif_object *object)
> }
>
> static int
> -usif_object_new(struct drm_file *f, void *data, u32 size, void *argv, u32
> argc)
> +usif_object_new(struct drm_file *f, void *data, u32 size, void *argv, u32
> argc, bool parent_abi16)
> {
> struct nouveau_cli *cli = nouveau_cli(f);
> struct nvif_client *client = &cli->base;
> @@ -271,23 +274,48 @@ usif_object_new(struct drm_file *f, void *data, u32
> size, void *argv, u32 argc)
> struct usif_object *object;
> int ret = -ENOSYS;
>
> + if ((ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, true)))
> + return ret;
> +
> + switch (args->v0.oclass) {
> + case NV_DMA_FROM_MEMORY:
> + case NV_DMA_TO_MEMORY:
> + case NV_DMA_IN_MEMORY:
> + return -EINVAL;
> + case NV_DEVICE: {
> + union {
> + struct nv_device_v0 v0;
> + } *args = data;
> +
> + if ((ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0,
> false)))
> + return ret;
> +
> + args->v0.priv = false;
> + break;
> + }
> + default:
> + if (!parent_abi16)
> + return -EINVAL;
> + break;
> + }
> +
> if (!(object = kmalloc(sizeof(*object), GFP_KERNEL)))
> return -ENOMEM;
> list_add(&object->head, &cli->objects);
>
> - if (!(ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, true))) {
> - object->route = args->v0.route;
> - object->token = args->v0.token;
> - args->v0.route = NVDRM_OBJECT_USIF;
> - args->v0.token = (unsigned long)(void *)object;
> - ret = nvif_client_ioctl(client, argv, argc);
> - args->v0.token = object->token;
> - args->v0.route = object->route;
> + object->route = args->v0.route;
> + object->token = args->v0.token;
> + args->v0.route = NVDRM_OBJECT_USIF;
> + args->v0.token = (unsigned long)(void *)object;
> + ret = nvif_client_ioctl(client, argv, argc);
> + if (ret) {
> + usif_object_dtor(object);
> + return ret;
> }
>
> - if (ret)
> - usif_object_dtor(object);
> - return ret;
> + args->v0.token = object->token;
> + args->v0.route = object->route;
> + return 0;
> }
>
> int
> @@ -301,6 +329,7 @@ usif_ioctl(struct drm_file *filp, void __user *user, u32
> argc)
> struct nvif_ioctl_v0 v0;
> } *argv = data;
> struct usif_object *object;
> + bool abi16 = false;
> u8 owner;
> int ret;
>
> @@ -331,11 +360,13 @@ usif_ioctl(struct drm_file *filp, void __user *user,
> u32 argc)
> mutex_unlock(&cli->mutex);
> goto done;
> }
> +
> + abi16 = true;
> }
>
> switch (argv->v0.type) {
> case NVIF_IOCTL_V0_NEW:
> - ret = usif_object_new(filp, data, size, argv, argc);
> + ret = usif_object_new(filp, data, size, argv, argc, abi16);
> break;
> case NVIF_IOCTL_V0_NTFY_NEW:
> ret = usif_notify_new(filp, data, size, argv, argc);
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
> index 513ee6b79553..08100eed9584 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
> @@ -347,7 +347,7 @@ nvkm_udevice_new(const struct nvkm_oclass *oclass, void
> *data, u32 size,
> return ret;
>
> /* give priviledged clients register access */
> - if (client->super)
> + if (args->v0.priv)
> func = &nvkm_udevice_super;
> else
> func = &nvkm_udevice;
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat