Le 4/7/22 à 12:32, Peter Maydell a écrit :
On Tue, 14 Jun 2016 at 15:40, Peter Maydell <peter.mayd...@linaro.org> wrote:
From: KONRAD Frederic <fred.kon...@greensocs.com>
This is the implementation of the DisplayPort.
It has an aux-bus to access dpcd and edid.
Graphic plane is connected to the channel 3.
Video plane is connected to the channel 0.
Audio stream are connected to the channels 4 and 5.
Very old patch, but Coverity has just pointed out an array
overrun in it (CID 1487260):
We define a set of offsets for V_BLEND registers, of which
the largest is this one:
+#define V_BLEND_CHROMA_KEY_COMP3 (0x01DC >> 2)
+static void xlnx_dp_vblend_write(void *opaque, hwaddr offset,
+ uint64_t value, unsigned size)
+{
+ XlnxDPState *s = XLNX_DP(opaque);
+ bool alpha_was_enabled;
+
+ DPRINTF("vblend: write @0x%" HWADDR_PRIX " = 0x%" PRIX32 "\n", offset,
+
(uint32_t)value);
+ offset = offset >> 2;
+
+ switch (offset) {
+ case V_BLEND_CHROMA_KEY_COMP1:
+ case V_BLEND_CHROMA_KEY_COMP2:
+ case V_BLEND_CHROMA_KEY_COMP3:
+ s->vblend_registers[offset] = value & 0x0FFF0FFF;
We use V_BLEND_CHROMA_KEY_COMP3 as an index into the vblend_registers array...
+ break;
+ default:
+ s->vblend_registers[offset] = value;
+ break;
+ }
+}
+#define DP_CORE_REG_ARRAY_SIZE (0x3AF >> 2)
+#define DP_AVBUF_REG_ARRAY_SIZE (0x238 >> 2)
+#define DP_VBLEND_REG_ARRAY_SIZE (0x1DF >> 2)
+#define DP_AUDIO_REG_ARRAY_SIZE (0x50 >> 2)
+ uint32_t vblend_registers[DP_VBLEND_REG_ARRAY_SIZE];
..but we have defined DP_VBLEND_REG_ARRAY_SIZE to 0x1DF >> 2,
which is the same as 0x1DC >> 2, and so the array size is too small.
The size of the memory region is also suspicious:
+ memory_region_init_io(&s->vblend_iomem, obj, &vblend_ops, s, TYPE_XLNX_DP
+ ".v_blend", 0x1DF);
This is a "32-bit accesses only" region, but we have defined it with a
size that is not a multiple of 4. That looks wrong... (It also means
that rather than having an array overrun I think the actual effect
is that the guest won't be able to access the last register, because
it's not entirely within the memoryregion.)
arg, sorry for that..
I share your point, it should not be possible to access it, but using
the monitor:
(qemu) info mtree
...
00000000fd4aa000-00000000fd4aa1de (prio 0, i/o): xlnx.v-dp.v_blend
...
I can actually read that register (at least it doesn't complain, on an
older qemu version though):
(qemu) xp /w 0xfd4aa1dc
00000000fd4aa1dc: 0x00000000
So I'm not totally sure.. do you need a patch for 7.0.0?
Coverity doesn't complain about it, but the DP_CORE_REG_ARRAY_SIZE
may also have a similar problem.
I think it doesn't complain because writing to the last register doesn't
actually write into the array but update the mask register instead:
case DP_INT_DS:
s->core_registers[DP_INT_MASK] |= ~value;
xlnx_dp_update_irq(s);
break;
thanks
-- PMM
Best Regards,
Fred
