ps2: check PS2Queue indices in post_load routine
PATCH v2 1/1 ps2: check PS2Queue indices in post_load routine
From: Prasad J Pandit <[email protected]>
During Qemu guest migration, a destination process invokes ps2 post_load function. In that, if 'rptr' and 'count' values were invalid, it could lead to OOB access issue. Add check to avoid it.
--- hw/input/ps2.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-)
Update: return -1(bad indices) error, instead of correcting values
-> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg02964.html
hw/input/ps2.c +18 -21
@@ -1222,32 +1222,37 @@ static void ps2_common_reset(PS2State *s) s->update_irq (s->update_arg,0 ); } -static void ps2_common_post_load (PS2State *s) +static int ps2_common_post_load (PS2State *s) {PS2Queue *q = &s->queue;int size;int i;int tmp_data[PS2_QUEUE_SIZE ]; +if (q->count <0 || q->rptr <0 || q->rptr >=sizeof (q->data)) { +return -1 ; + } +/* set the useful data buffer queue size, < PS2_QUEUE_SIZE */ size = q->count >PS2_QUEUE_SIZE ?0 : q->count;/* move the queue elements to the start of data array */ -if (size >0 ) { -for (i =0 ; i < size; i++) { -/* move the queue elements to the temporary buffer */ - tmp_data[i] = q->data[q->rptr]; -if (++q->rptr ==256 ) { - q->rptr =0 ; - } +for (i =0 ; i < size; i++) { +/* move the queue elements to the temporary buffer */ + tmp_data[i] = q->data[q->rptr]; +if (++q->rptr ==256 ) { + q->rptr =0 ; } -memcpy (q->data, tmp_data, size); } +memcpy (q->data, tmp_data, size); +/* reset rptr/wptr/count */ q->rptr =0 ; q->wptr = size; q->count = size; s->update_irq (s->update_arg, q->count !=0 ); + +return 0 ; }static void ps2_kbd_reset (void *opaque)
@@ -1346,9 +1351,7 @@ static int ps2_kbd_post_load(void* opaque, int version_id)if (version_id ==2 ) s->scancode_set=2 ; -ps2_common_post_load (ps2); - -return 0 ; +return ps2_common_post_load (ps2); }static int ps2_kbd_pre_save (void *opaque)
@@ -1356,9 +1359,7 @@ static int ps2_kbd_pre_save(void *opaque)PS2KbdState *s = (PS2KbdState *)opaque;PS2State *ps2 = &s->common; -ps2_common_post_load (ps2); - -return 0 ; +return ps2_common_post_load (ps2); }static const VMStateDescription vmstate_ps2_keyboard = {
@@ -1386,9 +1387,7 @@ static int ps2_mouse_post_load(void *opaque, int version_id)PS2MouseState *s = (PS2MouseState *)opaque;PS2State *ps2 = &s->common; -ps2_common_post_load (ps2); - -return 0 ; +return ps2_common_post_load (ps2); }static int ps2_mouse_pre_save (void *opaque)
@@ -1396,9 +1395,7 @@ static int ps2_mouse_pre_save(void *opaque)PS2MouseState *s = (PS2MouseState *)opaque;PS2State *ps2 = &s->common; -ps2_common_post_load (ps2); - -return 0 ; +return ps2_common_post_load (ps2); }static const VMStateDescription vmstate_ps2_mouse = {
Signature · 1 line
2.13.6
Quoted 102 lines · P J P
From: Prasad J Pandit <[email protected]>
During Qemu guest migration, a destination process invokes ps2
post_loadfunction. In that, if 'rptr' and 'count' values were invalid, it could lead to OOB access issue. Add check to avoid it.Reported-by Cyrille Chatras <[email protected]>Signed-off-by Prasad J Pandit <[email protected]>--- hw/input/ps2.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-)Update: return -1(bad indices) error, instead of correcting values
-> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg02964.htmlhw/input/ps2.c +17
-18@@ -1222,32 +1222,37 @@ static void ps2_common_reset(PS2State *s) s->update_irq (s->update_arg,0 ); } -static void ps2_common_post_load (PS2State *s) +static int ps2_common_post_load (PS2State *s) {PS2Queue *q = &s->queue;int size;int i;int tmp_data[PS2_QUEUE_SIZE ]; +if (q->count <0 || q->rptr <0 || q->rptr >=sizeof (q->data)) { +return -1 ; + } +/* set the useful data buffer queue size, < PS2_QUEUE_SIZE */ size = q->count >PS2_QUEUE_SIZE ?0 : q->count;/* move the queue elements to the start of data array */ -if (size >0 ) { -for (i =0 ; i < size; i++) { -/* move the queue elements to the temporary buffer */ - tmp_data[i] = q->data[q->rptr]; -if (++q->rptr ==256 ) { - q->rptr =0 ; - } +for (i =0 ; i < size; i++) { +/* move the queue elements to the temporary buffer */ + tmp_data[i] = q->data[q->rptr]; +if (++q->rptr ==256 ) { + q->rptr =0 ; } -memcpy (q->data, tmp_data, size); } +memcpy (q->data, tmp_data, size); +/* reset rptr/wptr/count */ q->rptr =0 ; q->wptr = size; q->count = size; s->update_irq (s->update_arg, q->count !=0 ); + +return 0 ; }static void ps2_kbd_reset (void *opaque)@@ -1346,9 +1351,7 @@ static int ps2_kbd_post_load(void* opaque, int version_id)if (version_id ==2 ) s->scancode_set=2 ; -ps2_common_post_load (ps2); - -return 0 ; +return ps2_common_post_load (ps2); }static int ps2_kbd_pre_save (void *opaque)@@ -1356,9 +1359,7 @@ static int ps2_kbd_pre_save(void *opaque)PS2KbdState *s = (PS2KbdState *)opaque;PS2State *ps2 = &s->common; -ps2_common_post_load (ps2); - -return 0 ; +return ps2_common_post_load (ps2); }static const VMStateDescription vmstate_ps2_keyboard = {@@ -1386,9 +1387,7 @@ static int ps2_mouse_post_load(void *opaque, int version_id)PS2MouseState *s = (PS2MouseState *)opaque;PS2State *ps2 = &s->common; -ps2_common_post_load (ps2); - -return 0 ; +return ps2_common_post_load (ps2); }static int ps2_mouse_pre_save (void *opaque)
hw/input/ps2.c static int ps2_mouse_pre_save(void *opaque) · line 1401
Quoted 10 lines
hw/input/ps2.c +1
-3@@ -1396,9 +1395,7 @@ static int ps2_mouse_pre_save(void *opaque)PS2MouseState *s = (PS2MouseState *)opaque;PS2State *ps2 = &s->common; -ps2_common_post_load (ps2); - -return 0 ; +return ps2_common_post_load (ps2); }static const VMStateDescription vmstate_ps2_mouse = {