On Fri, Oct 20, 2006 at 06:19:43PM -0700, Jouni Malinen wrote:
> On Fri, Oct 20, 2006 at 06:20:15PM -0400, Pavel Roskin wrote:
> 
> > The record length for numerical manufacturer ID should be at least 4
> > bytes (two 16-bit words).  The code required 5 bytes, which would break
> > for most (if not all) cards.  Reported by [EMAIL PROTECTED]
> 
> >             case CISTPL_MANFID:
> > -                   if (cis[pos + 1] < 5)
> > +                   if (cis[pos + 1] < 4)
> 
> Hmm.. Interesting. I think this was changed from 4 to 5 due to a
> potential buffer overflow as reported by Coverity tools.. In addition, I
> think that I spent long time trying to understand why it could be a
> buffer overflow and since it was changed, likely finally figured out an
> example case.. Alas, I don't remember what exactly this was anymore.
> 
> It looks like the comparison of the length field to be <5 was incorrect,
> but in order to avoid re-introducing any potential buffer overflows,
> that condition could be extended to verify that pos is small enough..
> Something like (cis[pos + 1] < 4 && pos + 5 < CIS_MAX_LEN) could be a
> better fix here. I don't have easy access to PLX cards anymore, so this
> is untested and I'm too lazy to copy this function into a separate
> program to run it against CIS dumps.

Pavel,

Will you be refactoring this patch?  Or do you disagree with Jouni's
assessment?

Thanks,

John
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to