[alsa-devel] Suspected heap overflow in snd_midi_event_encode_byte()
Mike Frysinger
vapier at chromium.org
Tue Aug 28 22:08:53 CEST 2018
the preferred flow looks something like:
- we put together the fix
- send fix to upstream (alsa) project
- upstream reviews/merges
- if we get a new release, update to that
- if new release is going to be a while, backport the patch to latest
ebuild and send a PR to Gentoo's GH (github.com/gentoo/gentoo)
- once Gentoo merges the PR, roll the fix into portage-stable
otherwise if things are up for debate along the way, we have
chromiumos-overlay to hold our own forked packages.
-mike
On Mon, Aug 27, 2018 at 8:33 AM Takashi Iwai <tiwai at suse.de> wrote:
> On Thu, 23 Aug 2018 22:41:15 +0200,
> Prashant Malani wrote:
> >
> > Thanks.
> >
> > Curious to know: Whats the URL for alsa-lib git tree? I'd love to be able
> > to check it out.
>
> You can get it from git.alsa-project.org
>
>
> Takashi
>
> >
> > On Wed, Aug 22, 2018 at 11:46 PM, Takashi Iwai <tiwai at suse.de> wrote:
> >
> > > On Wed, 22 Aug 2018 23:32:35 +0200,
> > > Prashant Malani wrote:
> > > >
> > > > Thanks Takashi,
> > > >
> > > > I can confirm that this patch fixes the heap overflow issue.
> > > > Could you kindly submit this patch into the alsa-lib tree?
> > >
> > > Now applied the following patch to git tree.
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > > -- 8< --
> > > From: Takashi Iwai <tiwai at suse.de>
> > > Subject: [PATCH] seq: Fix signedness in MIDI encoder/decoder
> > >
> > > The qlen field of struct snd_midi_event was declared as size_t while
> > > status_events[] assigns the qlen to -1 indicating to skip. This leads
> > > to the misinterpretation since size_t is unsigned, hence it passes the
> > > check "dev.qlen > 0" incorrectly in snd_midi_event_encode_byte(),
> > > which eventually results in a memory corruption.
> > >
> > > Also, snd_midi_event_decode() doesn't consider about a negative qlen
> > > value and tries to copy the size as is.
> > >
> > > This patch fixes these issues: the first one is addressed by simply
> > > replacing size_t with ssize_t in snd_midi_event struct. For the
> > > latter, a check "qlen <= 0" is added to bail out; this is also good as
> > > a slight optimization.
> > >
> > > Reported-by: Prashant Malani <pmalani at chromium.org>
> > > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > > ---
> > > src/seq/seq_midi_event.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/seq/seq_midi_event.c b/src/seq/seq_midi_event.c
> > > index 2e7d1035442a..5a12a18ce781 100644
> > > --- a/src/seq/seq_midi_event.c
> > > +++ b/src/seq/seq_midi_event.c
> > > @@ -35,7 +35,7 @@
> > >
> > > /* midi status */
> > > struct snd_midi_event {
> > > - size_t qlen; /* queue length */
> > > + ssize_t qlen; /* queue length */
> > > size_t read; /* chars read */
> > > int type; /* current event type */
> > > unsigned char lastcmd;
> > > @@ -606,6 +606,8 @@ long snd_midi_event_decode(snd_midi_event_t *dev,
> > > unsigned char *buf, long count
> > > status_event[type].decode(ev, xbuf +
> 0);
> > > qlen = status_event[type].qlen;
> > > }
> > > + if (qlen <= 0)
> > > + return 0;
> > > if (count < qlen)
> > > return -ENOMEM;
> > > memcpy(buf, xbuf, qlen);
> > > --
> > > 2.18.0
> > >
> > >
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel at alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
>
More information about the Alsa-devel
mailing list