lkml.org 
[lkml]   [2016]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] ALSA: usb-audio: Fix double-free in snd_usb_add_audio_stream()
On Thu, 31 Mar 2016 16:03:55 +0200,
Vladis Dronov wrote:
>
> Hello, Takashi, all,
>
> > No, it has nothing to do with the double-free bug itself. Such an
> > optimization shouldn't be put in a fix patch
>
> This piece of code move alone fixes the double-free bug in
> create_fixed_stream_quirk(), so I believe it is related.

The merits you pointed are irrelevant from the double-free bug.

> Besides, a lot of stuff
> is created and initialized in snd_usb_add_audio_stream() and while I do not see
> another use-after-free bug, it could be there. By moving this code we avoid
> these potential bugs we have not hit yet.

It's a different issue. The only judgment now is which one is clearer
to understand. The code efficiency isn't a question for this bug,
since the error condition is very rare, and it's no hot path, after
all.

> But anyway. If you still believe this code should not be moved, please, confirm,
> I'll suggest the next patch version without it.

Right, I don't want to move it.


> > Vladis, if you take someone's patch as the base, you have to carry the
> > original authorship somewhere...
>
> Yes, I was thinking about it, I was just not sure how should I do it. Will the
> following form be fine? Or somehow else?
>
> Based on a patch by Takashi Iwai" <tiwai@suse.de>

Yes, usually such a line should be enough.


> > > + * if not, create a new pcm stream. the caller must remove fp from
> > > + * the substream fmt_list in the error path to avoid double-free.
> >
> > This comment isn't true. The caller may leave it as is, too, like my
> > first version. It just depends on the code.
>
> Yes. Is the following rewrite acceptable for the next patch version?
>
> * if not, create a new pcm stream. Note, fp is added to the substream fmt_list
> * and will be freed on the chip instance release. Do not free fp or do remove
> * it from the substream fmt_list to avoid double-free.

Yes, that looks more correct.


thanks,

Takashi

\
 
 \ /
  Last update: 2016-03-31 16:41    [W:0.073 / U:0.724 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site