diff mbox series

es1370: check total frame count against current frame

Message ID 20200514200608.1744203-1-ppandit@redhat.com (mailing list archive)
State New, archived
Headers show
Series es1370: check total frame count against current frame | expand

Commit Message

Prasad Pandit May 14, 2020, 8:06 p.m. UTC 
From: Prasad J Pandit <pjp@fedoraproject.org>

A guest user may set channel frame count via es1370_write()
such that, in es1370_transfer_audio(), total frame count
'size' is lesser than the number of frames that are processed
'cnt'.

    int cnt = d->frame_cnt >> 16;
    int size = d->frame_cnt & 0xffff;

if (size < cnt), it results in incorrect calculations leading
to OOB access issue(s). Add check to avoid it.

Reported-by: Ren Ding <rding@gatech.edu>
Reported-by: Hanqing Zhao <hanqing@gatech.edu>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/audio/es1370.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Prasad Pandit May 19, 2020, 6:35 p.m. UTC | #1 
+-- On Fri, 15 May 2020, P J P wrote --+
| From: Prasad J Pandit <pjp@fedoraproject.org>
| 
| A guest user may set channel frame count via es1370_write()
| such that, in es1370_transfer_audio(), total frame count
| 'size' is lesser than the number of frames that are processed
| 'cnt'.
| 
|     int cnt = d->frame_cnt >> 16;
|     int size = d->frame_cnt & 0xffff;
| 
| if (size < cnt), it results in incorrect calculations leading
| to OOB access issue(s). Add check to avoid it.
| 

Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
Gerd Hoffmann May 20, 2020, 8:56 a.m. UTC | #2 
On Wed, May 20, 2020 at 12:05:48AM +0530, P J P wrote:
> +-- On Fri, 15 May 2020, P J P wrote --+
> | From: Prasad J Pandit <pjp@fedoraproject.org>
> | 
> | A guest user may set channel frame count via es1370_write()
> | such that, in es1370_transfer_audio(), total frame count
> | 'size' is lesser than the number of frames that are processed
> | 'cnt'.
> | 
> |     int cnt = d->frame_cnt >> 16;
> |     int size = d->frame_cnt & 0xffff;
> | 
> | if (size < cnt), it results in incorrect calculations leading
> | to OOB access issue(s). Add check to avoid it.
> | 
> 
> Ping...!

Added to audio patch queue.

(there isn't much activity in audio, thats why the mail was sitting in
my mailbox waiting for me process it ...)

thanks,
  Gerd
diff mbox series

Patch

diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 89c4dabcd4..5f8a83ff56 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -643,6 +643,9 @@  static void es1370_transfer_audio (ES1370State *s, struct chan *d, int loop_sel,
     int csc_bytes = (csc + 1) << d->shift;
     int cnt = d->frame_cnt >> 16;
     int size = d->frame_cnt & 0xffff;
+    if (size < cnt) {
+        return;
+    }
     int left = ((size - cnt + 1) << 2) + d->leftover;
     int transferred = 0;
     int temp = MIN (max, MIN (left, csc_bytes));
@@ -651,7 +654,7 @@  static void es1370_transfer_audio (ES1370State *s, struct chan *d, int loop_sel,
     addr += (cnt << 2) + d->leftover;
 
     if (index == ADC_CHANNEL) {
-        while (temp) {
+        while (temp > 0) {
             int acquired, to_copy;
 
             to_copy = MIN ((size_t) temp, sizeof (tmpbuf));
@@ -669,7 +672,7 @@  static void es1370_transfer_audio (ES1370State *s, struct chan *d, int loop_sel,
     else {
         SWVoiceOut *voice = s->dac_voice[index];
 
-        while (temp) {
+        while (temp > 0) {
             int copied, to_copy;
 
             to_copy = MIN ((size_t) temp, sizeof (tmpbuf));