On Thu, Mar 17, 2011 at 3:23 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > On Thu, Mar 17, 2011 at 01:45:33PM +0800, Xiaotian Feng wrote: > >> I guess we need also switch pstore from ->get_sb() to ->mount() >> >> fs/pstore/inode.c:253: error: unknown field ???get_sb??? specified in initializer >> fs/pstore/inode.c:253: warning: initialization makes integer from >> pointer without a cast >> fs/pstore/inode.c:253: error: initializer element is not computable at load time >> fs/pstore/inode.c:253: error: (near initialization for >> ???pstore_fs_type.fs_flags???) > > Yes. ÂI've just looked at that thing and I see several, er, issues. > Cc'ed author ... > a) What the hell would you expect to happen if userland mounts it twice > and unmount the first one? Âpstore_sb = NULL, pstore_mnt = NULL, AFAICS. > > b) pstore_writefile() - struct file on stack? ÂReally? ÂAgain, in the > scenario above, what'll happen to you if pstore_mnt gets dropped and > freed in the middle of all that? > > c) in the same function: > + Â Â Â memset(&f, '0', sizeof f); > Ahem... > > d) > + Â Â Â Â Â Â Â if (pstore_is_mounted()) > + Â Â Â Â Â Â Â Â Â Â Â pstore_mkfile(PSTORE_TYPE_DMESG, psinfo->name, id, > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â psinfo->buf, hsize + l1_cpy + l2_cpy, > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â CURRENT_TIME, psinfo->erase); > > And what happens if it's unmounted between the check and use? Âpstore_mkfile() > pretty much starts with dereferencing pstore_sb... > > e) as the matter of fact, what happens if it's unmounted in the _middle_ of > pstore_mkfile()? > > f) in general it's a lousy policy to tie that kind of difference in behaviour > to "somebody has it mounted someplace at the moment", even without the races > a-la (a)... > -- 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
