Re: kernel BUG with 2.6.23-rc3-mm1: skb_over_panic

[Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>]

* Greg KH (greg@kroah.com) wrote:
> On Fri, Aug 24, 2007 at 05:44:50PM -0700, Andrew Morton wrote:
> > On Fri, 24 Aug 2007 20:16:38 -0400
> > Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> > 
> > > * Andrew Morton (akpm@linux-foundation.org) wrote:
> > > > On Fri, 24 Aug 2007 18:47:07 -0400
> > > > Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> > > > 
> > > > > Hi Andrew,
> > > > > 
> > > > > I get the following BUG when booting 2.6.23-rc3-mm1 on i386. I wonder if
> > > > > you would have some ideas about what is causing this problem. I'll start
> > > > > bissecting it soon. I seems to be caused by an buggy skb_put call in
> > > > > kobject_uevent_env.
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > Mathieu
> > > > > 
> > > > > 
> > > > 
> > > > hm, don't know, sorry.  Kay fixed a few things in there, but iirc pretty
> > > > much all of the fixes were in rc3-mm1 anyway.
> > > > 
> > > > I doubt if bisection will tell us a lot: it'll probably point at
> > > > gregkh-driver-driver-core-change-add_uevent_var-to-use-a-struct.patch.
> > > > 
> > > > What we _would_ like to know is which sysfs file is being written to.  We
> > > > used to have a debug patch to exactly address this problem but it got
> > > > transferred into Greg's tree from whence it mysteriously disappeared.
> > > > 
> > > 
> > > Ok, here it is:
> > > 
> > > filename :
> > > 
> > > /devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/rev
> > 
> > Bah.  I've never found a sane way of going from a sysfs pathname back to the
> > code which implements that pathname :(
> > 
> > <greps the tree for '"rev"'>
> > 
> > <comes up with zilch>
> 
> It's a scsi file, as the above is a scsi device.  It's created in the
> drivers/scsi/scsi_sysfs.c file.
> 
> Kay, did you miss this set of attributes somehow?
> 
> thanks,
> 
> greg k-h

Hi Greg,

I think I am slowly getting there.. it looks like an off-by-one in
lib/kobject_uevent.c: add_uevent_var

when testing the return value of vsnprintf

if (len + 1 >= (sizeof(env->buf) - env->buflen))

should be

if (len >= (sizeof(env->buf) - env->buflen))

And then the problem underneath is that the array is too short for some
values. Since the return value of add_uevent_var is always ignored (why?)
from its callers, fixing the off-by-one will just fail silently, which is
almost worse.

I think we should find some better way of handling full static arrays.

And the bug is still there even if I fix these. So I'll continue my
investigation.

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68