On Tue, Oct 24, 2006 at 10:12:24PM -0400, Pavel Roskin wrote: > Coverity has no means to interpret CIS. However, it may understand > kmalloc, which allocates CIS_MAX_LEN for the CIS copy. > > The value of cis[pos + 1] has no bearing on the validity of the access > to cis[pos + 5] from the point of view of a checking tool.
Indeed. > pos is already checked in the beginning of the loop to be small enough, > but the check is not strong enough. The next tuple starts at (pos + > cis[pos + 1] + 2), and we want that to be at most CIS_MAX_LEN. > > That's something Coverity could have found. Agreed and I went through the report at some point and I think I found it to be valid.. > So, the right fix would be: > - if (pos + cis[pos + 1] >= CIS_MAX_LEN) > + if (pos + 2 + cis[pos + 1] > CIS_MAX_LEN) > goto cis_error; > I'm rewriting this with "<" because it's easier to read. Next tuple at > exactly CIS_MEX_LEN is fine - we just stop precessing at that point. > > I'm going to combine this with my previous fix and resend. Great, thanks! -- Jouni Malinen PGP id EFC895FA - 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
