Messages in this thread | | | Date | Fri, 30 Apr 2004 04:34:08 +0100 | From | viro@parcelfa ... | Subject | Re: [PATCH 2.4] add SMBIOS information to /proc/smbios -- UPDATED |
| |
On Thu, Apr 29, 2004 at 09:21:52PM -0500, Michael Brown wrote: > + u32 fp = 0xF0000; > + while (fp < 0xFFFFF) { > + isa_memcpy_fromio(table_eps, fp, sizeof(*table_eps)); > + if (memcmp(table_eps->anchor, "_SM_", 4)==0 && > + checksum_eps(table_eps)) { > + return 0; > + } > + fp += 16; > + }
Stilistic note: for (fp = 0xf0000; fp < 0xfffff; fp += 16) { isa_memcpy_fromio(table_eps, fp, sizeof(*table_eps)); if (memcmp(table_eps->anchor, "_SM_", 4) != 0) continue; if (checksum_eps(table_eps)) return 0; }
> + while(keep_going && ((ptr - buf) <= max_length) && count < max_count){ > + if (ptr[0] == 0x7F) /* ptr[0] is type */ > + keep_going = 0; > + > + ptr += ptr[1]; /* ptr[1] is length, skip structure */ > + /* skip strings at end of structure */ > + while((ptr-buf) < max_length && (ptr[0] || ptr[1])) > + ++ptr;
It looks like an off-by-one - if ptr reaches buf + max_length - 1, ptr[1] appears to be beyond the area it's OK to dereference.
> + size_t count, loff_t *ppos) > +{ > + unsigned long origppos = *ppos; > + unsigned long max_off = the_smbios_device.smbios_table_real_length; > + u8 *ptr; > + > + if(*ppos >= max_off) > + return 0;
Note that *ppos is signed here. llseek() to negative and you've got a problem.
> + while (*ppos < max_off) { > + put_user(readb(ptr + *ppos), buf); > + ++(*ppos); ++buf; > + }
Eeek...
a) that's called copy_to_user() b) you'd better check the return value (either of put_user() or copy_to_user()). - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |