Messages in this thread | | | Date | Thu, 17 Mar 2011 07:23:44 +0000 | From | Al Viro <> | Subject | Re: [git pull] mnt_devname queue |
| |
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.
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)...
| |