[linux/vd-agent,v1,7/7] vdagent: simple fix for address-of-packed-member

Submitted by Victor Toso on July 12, 2019, 9:12 a.m.

Details

Message ID 20190712091242.13214-8-victortoso@redhat.com
State Accepted
Commit 6bfbd03e83fc8fcf783d4431607d7d6129af58b0
Headers show
Series "minor fixes" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Victor Toso July 12, 2019, 9:12 a.m.
From: Victor Toso <me@victortoso.com>

Seems to be a false positive but as this message only happens when user
client connects, we can copy this array to make compiling warn free.

 > src/vdagent/vdagent.c: In function ‘daemon_read_complete’:
 > src/vdagent/vdagent.c:226:71: error: taking address of packed member of
 >     ‘struct VDAgentAudioVolumeSync’ may result in an unaligned pointer
 >     value [-Werror=address-of-packed-member]
 >   226 |             vdagent_audio_playback_sync(avs->mute, avs->nchannels, avs->volume);
 >       |                                                                    ~~~^~~~~~~~
 > src/vdagent/vdagent.c:228:69: error: taking address of packed member of
 >     ‘struct VDAgentAudioVolumeSync’ may result in an unaligned pointer
 >     value [-Werror=address-of-packed-member]
 >   228 |             vdagent_audio_record_sync(avs->mute, avs->nchannels, avs->volume);
 >       |                                                                  ~~~^~~~~~~~

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 src/vdagent/vdagent.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
index d799d1f..0e2e73e 100644
--- a/src/vdagent/vdagent.c
+++ b/src/vdagent/vdagent.c
@@ -222,11 +222,14 @@  static void daemon_read_complete(struct udscs_connection **connp,
         break;
     case VDAGENTD_AUDIO_VOLUME_SYNC: {
         VDAgentAudioVolumeSync *avs = (VDAgentAudioVolumeSync *)data;
+        uint16_t *volume = g_memdup(avs->volume, sizeof(uint16_t) * avs->nchannels);
+
         if (avs->is_playback) {
-            vdagent_audio_playback_sync(avs->mute, avs->nchannels, avs->volume);
+            vdagent_audio_playback_sync(avs->mute, avs->nchannels, volume);
         } else {
-            vdagent_audio_record_sync(avs->mute, avs->nchannels, avs->volume);
+            vdagent_audio_record_sync(avs->mute, avs->nchannels, volume);
         }
+        g_free(volume);
         break;
     }
     case VDAGENTD_FILE_XFER_DATA:

Comments

> 
> From: Victor Toso <me@victortoso.com>
> 
> Seems to be a false positive but as this message only happens when user
> client connects, we can copy this array to make compiling warn free.
> 
>  > src/vdagent/vdagent.c: In function ‘daemon_read_complete’:
>  > src/vdagent/vdagent.c:226:71: error: taking address of packed member of
>  >     ‘struct VDAgentAudioVolumeSync’ may result in an unaligned pointer
>  >     value [-Werror=address-of-packed-member]
>  >   226 |             vdagent_audio_playback_sync(avs->mute, avs->nchannels,
>  >   avs->volume);
>  >       |                                                                    ~~~^~~~~~~~
>  > src/vdagent/vdagent.c:228:69: error: taking address of packed member of
>  >     ‘struct VDAgentAudioVolumeSync’ may result in an unaligned pointer
>  >     value [-Werror=address-of-packed-member]
>  >   228 |             vdagent_audio_record_sync(avs->mute, avs->nchannels,
>  >   avs->volume);
>  >       |                                                                  ~~~^~~~~~~~
> 
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>  src/vdagent/vdagent.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index d799d1f..0e2e73e 100644
> --- a/src/vdagent/vdagent.c
> +++ b/src/vdagent/vdagent.c
> @@ -222,11 +222,14 @@ static void daemon_read_complete(struct
> udscs_connection **connp,
>          break;
>      case VDAGENTD_AUDIO_VOLUME_SYNC: {
>          VDAgentAudioVolumeSync *avs = (VDAgentAudioVolumeSync *)data;
> +        uint16_t *volume = g_memdup(avs->volume, sizeof(uint16_t) *
> avs->nchannels);
> +
>          if (avs->is_playback) {
> -            vdagent_audio_playback_sync(avs->mute, avs->nchannels,
> avs->volume);
> +            vdagent_audio_playback_sync(avs->mute, avs->nchannels, volume);
>          } else {
> -            vdagent_audio_record_sync(avs->mute, avs->nchannels,
> avs->volume);
> +            vdagent_audio_record_sync(avs->mute, avs->nchannels, volume);
>          }
> +        g_free(volume);
>          break;
>      }
>      case VDAGENTD_FILE_XFER_DATA:

This patch adds a buffer overflow.

Frediano
> 
> > 
> > From: Victor Toso <me@victortoso.com>
> > 
> > Seems to be a false positive but as this message only happens when user
> > client connects, we can copy this array to make compiling warn free.
> > 
> >  > src/vdagent/vdagent.c: In function ‘daemon_read_complete’:
> >  > src/vdagent/vdagent.c:226:71: error: taking address of packed member of
> >  >     ‘struct VDAgentAudioVolumeSync’ may result in an unaligned pointer
> >  >     value [-Werror=address-of-packed-member]
> >  >   226 |             vdagent_audio_playback_sync(avs->mute,
> >  >   avs->nchannels,
> >  >   avs->volume);
> >  >       |                                                                    ~~~^~~~~~~~
> >  > src/vdagent/vdagent.c:228:69: error: taking address of packed member of
> >  >     ‘struct VDAgentAudioVolumeSync’ may result in an unaligned pointer
> >  >     value [-Werror=address-of-packed-member]
> >  >   228 |             vdagent_audio_record_sync(avs->mute, avs->nchannels,
> >  >   avs->volume);
> >  >       |                                                                  ~~~^~~~~~~~
> > 
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > ---
> >  src/vdagent/vdagent.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> > index d799d1f..0e2e73e 100644
> > --- a/src/vdagent/vdagent.c
> > +++ b/src/vdagent/vdagent.c
> > @@ -222,11 +222,14 @@ static void daemon_read_complete(struct
> > udscs_connection **connp,
> >          break;
> >      case VDAGENTD_AUDIO_VOLUME_SYNC: {
> >          VDAgentAudioVolumeSync *avs = (VDAgentAudioVolumeSync *)data;
> > +        uint16_t *volume = g_memdup(avs->volume, sizeof(uint16_t) *
> > avs->nchannels);
> > +
> >          if (avs->is_playback) {
> > -            vdagent_audio_playback_sync(avs->mute, avs->nchannels,
> > avs->volume);
> > +            vdagent_audio_playback_sync(avs->mute, avs->nchannels,
> > volume);
> >          } else {
> > -            vdagent_audio_record_sync(avs->mute, avs->nchannels,
> > avs->volume);
> > +            vdagent_audio_record_sync(avs->mute, avs->nchannels, volume);
> >          }
> > +        g_free(volume);
> >          break;
> >      }
> >      case VDAGENTD_FILE_XFER_DATA:
> 
> This patch adds a buffer overflow.
> 

As a second though this is the protocol from daemon to agent so
source should be safe anyway.

Acked.

Frediano