Re: [uml-devel] [patch] mconsole_proc rewrite (and crash fix).

[BlaisorBlade <blaisorblade_spam@yahoo.it>]

Alle 22:40, lunedì 19 luglio 2004, Gerd Knorr ha scritto:
>   Hi,
>
> Below is a rewrite of the mconsole_proc function.

First: I'm collecting it inside my 2.6-bb patchset (which is then what I send 
for -mm inclusion).

Second: since there is in that tree the mconsole exec support (which allows 
executing any command including cat /proc/file > /dev/ttyN and cat /sys/*), 
which is as reliable as this version of mconsole_proc() but by far more 
general, I'm not sure if we should keep the proc command.

Actually the exec command cannot send anything to the mconsole, but since it 
invokes sh -c it allows for redirections to new consoles.

> The old code had the problem that the kernel crashed after calling
> "uml_mconsole proc <somefile>" a few times.  I havn't tracked what
> exactly causes the problem, I guess trying to access the procfs without
> actually mounting it somewhere causes some corruption of kernel data
> structures.

Who said that he didn't have mounted /proc inside the guest? I'm more 
convinced that while replicating the whole sys_open code Jeff didn't get 
something right. And it would have been worse if he ever managed to do it: it 
would have broken at the first little VFS change :-(. But it's only IMHO; on 
the other side, creating a kernel thread with his own namespace is safer but 
not cleaner than implementing sys_open ourselves.

However, I see basically two bugs:
1) why put_filesystem is not called at the end?
2) why don't we call dget() onto sb->s_root? I have some doubts about it, 
anyway.

> The new code simply openes /proc/<file> via sys_open().  That simplifies
> the function alot.  It also doesn't crash any more ;)
> Drawback is that it only works when procfs is actually mounted below
> /proc.  One suggestion I've received to fix the later issue was to mount
> the procfs within a kernel thread with a private namespace, but I havn't
> tried that so far.
I checked if kern_mount() would have worked but it's not straightforward, 
since it does not add the mounted fs anywhere.

However, sys_mount() and do_mount(), apart from argument checking, do a 
path_lookup(), to get a struct nameidata representing the mount point, and 
pass it to do_add_mount(). And only path_lookup() checks the current->fs 
datas, i.e. the root (not the ->namespace). Maybe that could help us, but I'm 
not sure...
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729



-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel