On 12/06/2015 01:56 AM, Al Viro wrote:
> Looks like exofs_new_inode() does this
>
> inode->i_ino = sbi->s_nextid++;
>
> without any locking; sure, the parent directory is locked, but that's not
> worth much on a filesystem that supports mkdir()... Am I missing something
> subtle here?
Yes I guess you are right. What bugs me is why this never failed. I mean
on a 64 bit system, why I never get a duplicate ino?
But I guess I should change this to an atomic.
> Another question in the code nearby:
>
> ret = ore_get_io_state(&sbi->layout, &oi->oc, &ios);
> if (unlikely(ret)) {
> EXOFS_ERR("exofs_new_inode: ore_get_io_state failed\n");
> return ERR_PTR(ret);
> }
>
> aren't we leaking a struct inode here? Path around ore_create() is
> also interesting - looks like its failure causes a leak (at least if
> it happens early on)...
>
Yes I'm afraid you are absolutely right. Just to show how much attention
this toy lab FS ever got. All ore_get_io_state needs to to fail is an OOM.
So this was never heavily tested right?
I'll see if I have some time to fix both. Just for the fun
Thanks Al
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
