Re: [PATCH 2.4] add SMBIOS information to /proc/smbios -- UPDATED

viro
2004-04-30T03:34:18Z

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()).