This patch workaround a possible security issue which can allow
user to abuse drm on r6xx/r7xx hw to access any system ram memory.
This patch doesn't break userspace, it detect "valid" old use of
CB_COLOR[0-7]_FRAG & CB_COLOR[0-7]_TILE registers and overwritte
the address these registers are pointing to with the one of the
last color buffer. This workaround will work for old mesa &
xf86-video-ati and any old user which did use similar register
programming pattern as those (we expect that there is no others
user of those ioctl except possibly a malicious one). This patch
add a warning if it detects such usage, warning encourage people
to update their mesa & xf86-video-ati. New userspace will submit
proper relocation.
Fix for xf86-video-ati / mesa (this kernel patch is enough to
prevent abuse, fix for userspace are to set proper cs stream and
avoid kernel warning) :
http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/commit/?id=95d63e408cc88b6934bec84a0b1ef94dfe8bee7b
http://cgit.freedesktop.org/mesa/mesa/commit/?id=46dc6fd3ed5ef96cda53641a97bc68c3bc104a9f
Abusing this register to perform system ram memory is not easy,
here is outline on how it could be achieve. First attacker must
have access to the drm device and be able to submit command stream
throught cs ioctl. Then attacker must build a proper command stream
for r6xx/r7xx hw which will abuse the FRAG or TILE buffer to
overwrite the GPU GART which is in VRAM. To achieve so attacker
as to setup CB_COLOR[0-7]_FRAG or CB_COLOR[0-7]_TILE to point
to the GPU GART, then it has to find a way to write predictable
value into those buffer (with little cleverness i believe this
can be done but this is an hard task). Once attacker have such
program it can overwritte GPU GART to program GPU gart to point
anywhere in system memory. It then can reusse same method as he
used to reprogram GART to overwritte the system ram through the
GART mapping. In the process the attacker has to be carefull to
not overwritte any sensitive area of the GART table, like ring
or IB gart entry as it will more then likely lead to GPU lockup.
Bottom line is that i think it's very hard to use this flaw
to get system ram access but in theory one can achieve so.
Side note: I am not aware of anyone ever using the GPU as an
attack vector, nevertheless we take great care in the opensource
driver to try to detect and forbid malicious use of GPU. I don't
think the closed source driver are as cautious as we are.
Signed-off-by: Jerome Glisse <[email protected]>
---
drivers/gpu/drm/radeon/r600_cs.c | 84 ++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/radeon/r600d.h | 25 +++++++++++
drivers/gpu/drm/radeon/radeon.h | 1 +
drivers/gpu/drm/radeon/radeon_cs.c | 1 +
4 files changed, 111 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c
index 44060b9..edafc7b 100644
--- a/drivers/gpu/drm/radeon/r600_cs.c
+++ b/drivers/gpu/drm/radeon/r600_cs.c
@@ -36,6 +36,10 @@ static int r600_cs_packet_next_reloc_nomm(struct radeon_cs_parser *p,
typedef int (*next_reloc_t)(struct radeon_cs_parser*, struct radeon_cs_reloc**);
static next_reloc_t r600_cs_packet_next_reloc = &r600_cs_packet_next_reloc_mm;
+struct r600_cs_track {
+ u32 cb_color0_base_last;
+};
+
/**
* r600_cs_packet_parse() - parse cp packet and point ib index to next packet
* @parser: parser structure holding parsing context.
@@ -177,6 +181,28 @@ static int r600_cs_packet_next_reloc_nomm(struct radeon_cs_parser *p,
}
/**
+ * r600_cs_packet_next_is_pkt3_nop() - test if next packet is packet3 nop for reloc
+ * @parser: parser structure holding parsing context.
+ *
+ * Check next packet is relocation packet3, do bo validation and compute
+ * GPU offset using the provided start.
+ **/
+static inline int r600_cs_packet_next_is_pkt3_nop(struct radeon_cs_parser *p)
+{
+ struct radeon_cs_packet p3reloc;
+ int r;
+
+ r = r600_cs_packet_parse(p, &p3reloc, p->idx);
+ if (r) {
+ return 0;
+ }
+ if (p3reloc.type != PACKET_TYPE3 || p3reloc.opcode != PACKET3_NOP) {
+ return 0;
+ }
+ return 1;
+}
+
+/**
* r600_cs_packet_next_vline() - parse userspace VLINE packet
* @parser: parser structure holding parsing context.
*
@@ -337,6 +363,7 @@ static int r600_packet3_check(struct radeon_cs_parser *p,
struct radeon_cs_packet *pkt)
{
struct radeon_cs_reloc *reloc;
+ struct r600_cs_track *track;
volatile u32 *ib;
unsigned idx;
unsigned i;
@@ -344,6 +371,7 @@ static int r600_packet3_check(struct radeon_cs_parser *p,
int r;
u32 idx_value;
+ track = (struct r600_cs_track *)p->track;
ib = p->ib->ptr;
idx = pkt->idx + 1;
idx_value = radeon_get_ib_value(p, idx);
@@ -503,9 +531,61 @@ static int r600_packet3_check(struct radeon_cs_parser *p,
for (i = 0; i < pkt->count; i++) {
reg = start_reg + (4 * i);
switch (reg) {
+ /* This register were added late, there is userspace
+ * which does provide relocation for those but set
+ * 0 offset. In order to avoid breaking old userspace
+ * we detect this and set address to point to last
+ * CB_COLOR0_BASE, note that if userspace doesn't set
+ * CB_COLOR0_BASE before this register we will report
+ * error. Old userspace always set CB_COLOR0_BASE
+ * before any of this.
+ */
+ case R_0280E0_CB_COLOR0_FRAG:
+ case R_0280E4_CB_COLOR1_FRAG:
+ case R_0280E8_CB_COLOR2_FRAG:
+ case R_0280EC_CB_COLOR3_FRAG:
+ case R_0280F0_CB_COLOR4_FRAG:
+ case R_0280F4_CB_COLOR5_FRAG:
+ case R_0280F8_CB_COLOR6_FRAG:
+ case R_0280FC_CB_COLOR7_FRAG:
+ case R_0280C0_CB_COLOR0_TILE:
+ case R_0280C4_CB_COLOR1_TILE:
+ case R_0280C8_CB_COLOR2_TILE:
+ case R_0280CC_CB_COLOR3_TILE:
+ case R_0280D0_CB_COLOR4_TILE:
+ case R_0280D4_CB_COLOR5_TILE:
+ case R_0280D8_CB_COLOR6_TILE:
+ case R_0280DC_CB_COLOR7_TILE:
+ if (!r600_cs_packet_next_is_pkt3_nop(p)) {
+ if (!track->cb_color0_base_last) {
+ dev_err(p->dev, "Broken old userspace ? no cb_color0_base supplied"
+ "before trying to write 0x%08X\n", reg);
+ return -EINVAL;
+ }
+ ib[idx+1+i] = track->cb_color0_base_last;
+ printk_once(KERN_WARNING "You have old & broken userspace "
+ "please consider updating mesa & xf86-video-ati\n");
+ } else {
+ r = r600_cs_packet_next_reloc(p, &reloc);
+ if (r) {
+ dev_err(p->dev, "bad SET_CONTEXT_REG 0x%04X\n", reg);
+ return -EINVAL;
+ }
+ ib[idx+1+i] += (u32)((reloc->lobj.gpu_offset >> 8) & 0xffffffff);
+ }
+ break;
case DB_DEPTH_BASE:
case DB_HTILE_DATA_BASE:
case CB_COLOR0_BASE:
+ r = r600_cs_packet_next_reloc(p, &reloc);
+ if (r) {
+ DRM_ERROR("bad SET_CONTEXT_REG "
+ "0x%04X\n", reg);
+ return -EINVAL;
+ }
+ ib[idx+1+i] += (u32)((reloc->lobj.gpu_offset >> 8) & 0xffffffff);
+ track->cb_color0_base_last = ib[idx+1+i];
+ break;
case CB_COLOR1_BASE:
case CB_COLOR2_BASE:
case CB_COLOR3_BASE:
@@ -678,8 +758,11 @@ static int r600_packet3_check(struct radeon_cs_parser *p,
int r600_cs_parse(struct radeon_cs_parser *p)
{
struct radeon_cs_packet pkt;
+ struct r600_cs_track *track;
int r;
+ track = kzalloc(sizeof(*track), GFP_KERNEL);
+ p->track = track;
do {
r = r600_cs_packet_parse(p, &pkt, p->idx);
if (r) {
@@ -757,6 +840,7 @@ int r600_cs_legacy(struct drm_device *dev, void *data, struct drm_file *filp,
/* initialize parser */
memset(&parser, 0, sizeof(struct radeon_cs_parser));
parser.filp = filp;
+ parser.dev = &dev->pdev->dev;
parser.rdev = NULL;
parser.family = family;
parser.ib = &fake_ib;
diff --git a/drivers/gpu/drm/radeon/r600d.h b/drivers/gpu/drm/radeon/r600d.h
index 05894ed..3048088 100644
--- a/drivers/gpu/drm/radeon/r600d.h
+++ b/drivers/gpu/drm/radeon/r600d.h
@@ -882,4 +882,29 @@
#define S_000E60_SOFT_RESET_VMC(x) (((x) & 1) << 17)
#define R_005480_HDP_MEM_COHERENCY_FLUSH_CNTL 0x5480
+
+#define R_0280E0_CB_COLOR0_FRAG 0x0280E0
+#define S_0280E0_BASE_256B(x) (((x) & 0xFFFFFFFF) << 0)
+#define G_0280E0_BASE_256B(x) (((x) >> 0) & 0xFFFFFFFF)
+#define C_0280E0_BASE_256B 0x00000000
+#define R_0280E4_CB_COLOR1_FRAG 0x0280E4
+#define R_0280E8_CB_COLOR2_FRAG 0x0280E8
+#define R_0280EC_CB_COLOR3_FRAG 0x0280EC
+#define R_0280F0_CB_COLOR4_FRAG 0x0280F0
+#define R_0280F4_CB_COLOR5_FRAG 0x0280F4
+#define R_0280F8_CB_COLOR6_FRAG 0x0280F8
+#define R_0280FC_CB_COLOR7_FRAG 0x0280FC
+#define R_0280C0_CB_COLOR0_TILE 0x0280C0
+#define S_0280C0_BASE_256B(x) (((x) & 0xFFFFFFFF) << 0)
+#define G_0280C0_BASE_256B(x) (((x) >> 0) & 0xFFFFFFFF)
+#define C_0280C0_BASE_256B 0x00000000
+#define R_0280C4_CB_COLOR1_TILE 0x0280C4
+#define R_0280C8_CB_COLOR2_TILE 0x0280C8
+#define R_0280CC_CB_COLOR3_TILE 0x0280CC
+#define R_0280D0_CB_COLOR4_TILE 0x0280D0
+#define R_0280D4_CB_COLOR5_TILE 0x0280D4
+#define R_0280D8_CB_COLOR6_TILE 0x0280D8
+#define R_0280DC_CB_COLOR7_TILE 0x0280DC
+
+
#endif
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index c965396..8deb8c1 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -464,6 +464,7 @@ struct radeon_cs_chunk {
};
struct radeon_cs_parser {
+ struct device *dev;
struct radeon_device *rdev;
struct drm_file *filp;
/* chunks */
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index 65590a0..1496cb8 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -231,6 +231,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
memset(&parser, 0, sizeof(struct radeon_cs_parser));
parser.filp = filp;
parser.rdev = rdev;
+ parser.dev = rdev->dev;
r = radeon_cs_parser_init(&parser, data);
if (r) {
DRM_ERROR("Failed to initialize parser !\n");
--
1.6.5.2
On Mon, Jan 18, 2010 at 1:01 PM, Jerome Glisse <[email protected]> wrote:> This patch workaround a possible security issue which can allow> user to abuse drm on r6xx/r7xx hw to access any system ram memory.[...]> diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c> index 44060b9..edafc7b 100644> --- a/drivers/gpu/drm/radeon/r600_cs.c> +++ b/drivers/gpu/drm/radeon/r600_cs.c> @@ -503,9 +531,61 @@ static int r600_packet3_check(struct radeon_cs_parser *p,> for (i = 0; i < pkt->count; i++) {> reg = start_reg + (4 * i);> switch (reg) {> + /* This register were added late, there is userspace> + * which does provide relocation for those but set> + * 0 offset. In order to avoid breaking old userspace> + * we detect this and set address to point to last> + * CB_COLOR0_BASE, note that if userspace doesn't set> + * CB_COLOR0_BASE before this register we will report> + * error. Old userspace always set CB_COLOR0_BASE> + * before any of this.> + */> + case R_0280E0_CB_COLOR0_FRAG:> + case R_0280E4_CB_COLOR1_FRAG:> + case R_0280E8_CB_COLOR2_FRAG:> + case R_0280EC_CB_COLOR3_FRAG:> + case R_0280F0_CB_COLOR4_FRAG:> + case R_0280F4_CB_COLOR5_FRAG:> + case R_0280F8_CB_COLOR6_FRAG:> + case R_0280FC_CB_COLOR7_FRAG:> + case R_0280C0_CB_COLOR0_TILE:> + case R_0280C4_CB_COLOR1_TILE:> + case R_0280C8_CB_COLOR2_TILE:> + case R_0280CC_CB_COLOR3_TILE:> + case R_0280D0_CB_COLOR4_TILE:> + case R_0280D4_CB_COLOR5_TILE:> + case R_0280D8_CB_COLOR6_TILE:> + case R_0280DC_CB_COLOR7_TILE:> + if (!r600_cs_packet_next_is_pkt3_nop(p)) {> + if (!track->cb_color0_base_last) {> + dev_err(p->dev, "Broken old userspace ? no cb_color0_base supplied"> + "before trying to write 0x%08X\n", reg);
Cosmetic issue: a space is missing between "supplied" and "before".
Luca????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?