Re: [PATCH 2.6] to fix i2o_proc kernel panic on access of /proc/i2o/iop0/lct

[Warren Togami <wtogami@redhat.com>]

Markus Lidel wrote:
> Hello,
> 
> Ingo Oeser wrote:
> 
>>> the patch fixes a bug in the i2o_proc.c module, where the kernel 
>>> panics, if you access /proc/i2o/iop0/lct and read more then 1024 
>>> bytes of it.
>>> The problem was, that no paging was implemented by the function. This 
>>> is now solved.
>>
>> What about solving this properly and using the seq_file API for this
>> problem class as anywhere else in the kernel?
>> Code will also get much more readable by this ;-)
> 
> 
> Didn't know of the seq_file API. But you are right, now it looks much 
> cleaner than before. Thanks for telling me!
> 

Thanks Markus!  This patch seems to solve the x86-64 rmmod oops. 
Unfortunately I have discovered another problem, a fix for that problem, 
then another problem after that.  I tested it by running the two scripts 
below simultaneously.  Not long after running the scripts it triggered 
many oopses and eventually a panic.

#!/bin/bash
date
modprobe i2o_proc
modprobe -r i2o_proc
exec $0

#!/bin/bash
cat /proc/i2o/iop0/lct
exec $0

http://togami.com/~warren/temp/i2o_proc.log.bz2
bzip2 compressed log from the netconsole, showing the many oops and 
tracebacks all the way to the final panic.  (I suspect this problem may 
exist in many other kernel modules too?)

--- kernel-2.6.5.orig/linux-2.6.5/drivers/message/i2o/i2o_proc.c 
2004-04-03 17:37:25.000000000 -1000
+++ kernel-2.6.5/linux-2.6.5/drivers/message/i2o/i2o_proc.c 
2004-05-09 01:18:05.667902973 -1000
@@ -3329,6 +3329,7 @@
         i2o_proc_dir_root = proc_mkdir("i2o", 0);
         if(!i2o_proc_dir_root)
                 return -1;
+       i2o_proc_dir_root->owner = THIS_MODULE;

         for(i = 0; i < MAX_I2O_CONTROLLERS; i++)
         {

Al Viro immediately saw the race condition as ->owner not being set. 
Adding the above patch seems to prevent oops and panic when 
simultaneously running the two scripts.  This test has been done on x86-64.

Unfortunately the modprobe script stops at a certain point when it 
attempts to load the module.  With the other script still looping, it 
appears to cause this:

[root@i2obox64 root]# lsmod
Module                  Size  Used by
i2o_proc               53056  4294961501

And dmesg repeats this message billions of times:
de_put: entry i2o already free!

I am guessing this is some kind of locking trouble?  In any case after 
this ->owner fix this module is no longer problematic, mainly because 
NOBODY has any good reason to be using the i2o_proc module (or load and 
unload it rapidly), and it is not necessary for normal i2o_block usage.

Warren Togami
wtogami@redhat.com