Re: [PATCH] one of $BIGNUM devfs races

[Alexander Viro]

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.