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
