Re: [PATCH] one of $BIGNUM devfs races

From: Alexander Viro (viro@math.psu.edu)
Date: Mon Aug 06 2001 - 20:42:56 EST


On Mon, 6 Aug 2001, Richard Gooch wrote:

> I'm referring specifically to this code:
> new->inode.ino = fs_info.num_inodes + FIRST_INODE;
> fs_info.table[fs_info.num_inodes++] = new;
>
> This is not SMP safe. Besides, even the allocation loop isn't SMP
> safe. If two tasks both allocate a table, they each could end up
> calling:
> kfree (fs_info.table);
> for the same value. Or for a different one (which is also bad).

BKL. kfree() is non-blocking. IOW, critical area can be placed under a
spinlock and BKL acts as such. We can trivially replace it with
a spinlock (static in function).

Actually, there is another problem with that code and it has nothing to
SMP. You never shrink that table and AFAICS you never reuse the entries.
IOW, you've got a leak there.

Why on the Earth do you need it, in the first place? Just put the
pointer to entry into inode->u.generic_ip and be done with that -
it kills all that mess for good. AFAICS the only places where you
really use that table is your get_devfs_entry_from_vfs_inode()
and devfs_write_inode(). In both cases pointer would be obviously more
convenient.

-
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/



This archive was generated by hypermail 2b29 : Tue Aug 07 2001 - 21:00:41 EST