Sasha Levin
2021-Aug-24 00:55 UTC
[Nouveau] [PATCH AUTOSEL 4.14 6/7] drm/nouveau: block a bunch of classes from userspace
From: Ben Skeggs <bskeggs at redhat.com>
[ 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 at redhat.com>
Reviewed-by: Lyude Paul <lyude at redhat.com>
Signed-off-by: Sasha Levin <sashal at kernel.org>
---
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;
--
2.30.2
Lyude Paul
2021-Aug-24 17:05 UTC
[Nouveau] [PATCH AUTOSEL 4.14 6/7] drm/nouveau: block a bunch of classes from userspace
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 at redhat.com> > > [ 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 at redhat.com> > Reviewed-by: Lyude Paul <lyude at redhat.com> > Signed-off-by: Sasha Levin <sashal at kernel.org> > --- > ?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