On Fri, 22 Nov 2024 14:01:53 +0100,
Benoît Sevens wrote:
>
> On Fri, 22 Nov 2024 at 13:09, Takashi Iwai <tiwai@xxxxxxx> wrote:
> >
> > On Fri, 22 Nov 2024 11:06:57 +0100,
> > Benoît Sevens wrote:
> > >
> > > Hi Takashi,
> > >
> > > Thank you for the review. Except for one question below, this patch
> > > looks good to me. Would you like me to send your patch as a revised
> > > patch to this mailing list?
> >
> > I'm going to submit the proper patch if the fix works for you.
> >
> > > On Thu, 21 Nov 2024 at 15:53, Takashi Iwai <tiwai@xxxxxxx> wrote:
> > > > @@ -65,13 +73,23 @@ static bool validate_clock_selector(void *p, int id, int proto)
> > > > {
> > > > union uac23_clock_selector_desc *cs = p;
> > > >
> > > > - return GET_VAL(cs, proto, bClockID) == id;
> > > > + if (!DESC_LENGTH_CHECK(cs, proto))
> > > > + return false;
> > > > + if (GET_VAL(cs, proto, bClockID) != id)
> > > > + return false;
> > > > + /* additional length check for baCSourceID, bmControls and iClockSelector */
> > > > + if (proto == UAC_VERSION_3)
> > > > + return cs->v3.bLength >= sizeof(cs->v3.bLength) + cs->v3.bNrInPins + 6;
> > >
> > > Why can't we just do this here?
> > >
> > > return cs->v3.bLength >= sizeof(cs->v3) + cs->v3.bNrInPins;
> >
> > Argh, it was just a wrong check.
> >
> > But the clock selector descriptor must have two more fields in
> > addition to baCSourceID array. Those two are 6 bytes (= 4 bytes for
> > bmControls + 2 bytes for wCSelectorDescrStr) for UAC3, while...
> >
> > > > + else
> > > > + return cs->v2.bLength >= sizeof(cs->v2.bLength) + cs->v2.bNrInPins + 2;
> > >
> > > And same question here, why not:
> > >
> > > return cs->v2.bLength >= sizeof(cs->v2) + cs->v2.bNrInPins;
> >
> > ... for UAC2, they are 2 bytes (= 1 byte for bmControls + 1 byte for
> > iClockSelector). So those numbers appeared in the patch.
> >
> > Below is the revised patch. Let me know if this works for you.
> >
>
> Looks perfect to me. Thank you for the patch and explanations!
>
> Can we cc stable to get it backported?
Sure, I'll add it when submitting properly.
thanks,
Takashi
[Index of Archives]
[Pulseaudio]
[Linux Audio Users]
[ALSA Devel]
[Fedora Desktop]
[Fedora SELinux]
[Big List of Linux Books]
[Yosemite News]
[KDE Users]