PATCH for NULL pointer in netback_uevent

PATCH for NULL pointer in netback_uevent

[James Harper]

The following is sufficient to get domain creation working on my server
(see threads "null-pointer access in netback_uevent" and "oops starting
domain on 4.0.0  + 2.6.32.13-gf6fe658"). I'm not sure if it's the right
solution though - should we expect a call to netback_uevent before the
vif is properly set up? All I'm doing is returning 0 (success?) if the
drvdata hasn't been set yet.

I also think that I am seeing a different problem to Bastian, based on
his debug output.

James

diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index 70636d0..a46d8b2 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -162,12 +162,16 @@ fail:
  */
 static int netback_uevent(struct xenbus_device *xdev, struct
kobj_uevent_env *env)
 {
-       struct backend_info *be = dev_get_drvdata(&xdev->dev);
-       struct xen_netif *netif = be->netif;
+       struct backend_info *be;
+       struct xen_netif *netif;
        char *val;

        DPRINTK("netback_uevent");

+       be = dev_get_drvdata(&xdev->dev);
+       if (!be)
+               return 0;
+       netif = be->netif;
        val = xenbus_read(XBT_NIL, xdev->nodename, "script", NULL);
        if (IS_ERR(val)) {
                int err = PTR_ERR(val);

Re: PATCH for NULL pointer in netback_uevent

[Jan Beulich]

>>> On 28.05.10 at 02:58, "James Harper" <james.harper@bendigoit.com.au> wrote:
> The following is sufficient to get domain creation working on my server
> (see threads "null-pointer access in netback_uevent" and "oops starting
> domain on 4.0.0  + 2.6.32.13-gf6fe658"). I'm not sure if it's the right
> solution though - should we expect a call to netback_uevent before the
> vif is properly set up? All I'm doing is returning 0 (success?) if the
> drvdata hasn't been set yet.

We've seen this just now too (with our non-pv-ops kernel). Since this
can be called due to sysfs reads (starting in 2.6.22), the function
must be prepared to be called at any time. I do think, however, that
it should provide as much data as possible in this state, i.e. not
plainly return zero in that case, but at least add the "script=" setting
(which is independent of "be" being NULL).

Even then we still depend on uevent not caching the output (but
rather re-issuing the read) once the backend for the new vif is fully
set up.

Jan

RE: PATCH for NULL pointer in netback_uevent

[James Harper]

> 
> >>> On 28.05.10 at 02:58, "James Harper"
<james.harper@bendigoit.com.au>
> wrote:
> > The following is sufficient to get domain creation working on my
server
> > (see threads "null-pointer access in netback_uevent" and "oops
starting
> > domain on 4.0.0  + 2.6.32.13-gf6fe658"). I'm not sure if it's the
right
> > solution though - should we expect a call to netback_uevent before
the
> > vif is properly set up? All I'm doing is returning 0 (success?) if
the
> > drvdata hasn't been set yet.
> 
> We've seen this just now too (with our non-pv-ops kernel). Since this
> can be called due to sysfs reads (starting in 2.6.22), the function
> must be prepared to be called at any time.

My assumption was that it happened because I upgraded udev which meant
turning off CONFIG_SYSFS_DEPRECATED_V2, and that one of those has
triggered the problem.

> I do think, however, that
> it should provide as much data as possible in this state, i.e. not
> plainly return zero in that case, but at least add the "script="
setting
> (which is independent of "be" being NULL).

Agreed. My patch got things up and running again for me, but I suspected
it wasn't really the correct approach.

> Even then we still depend on uevent not caching the output (but
> rather re-issuing the read) once the backend for the new vif is fully
> set up.
> 

I put debugging statements in and there were definitely multiple calls
to netback_uevent, if that counts for anything.

James

Re: PATCH for NULL pointer in netback_uevent

[Andrew Lyon]

On Fri, May 28, 2010 at 8:15 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>>> On 28.05.10 at 02:58, "James Harper" <james.harper@bendigoit.com.au> wrote:
>> The following is sufficient to get domain creation working on my server
>> (see threads "null-pointer access in netback_uevent" and "oops starting
>> domain on 4.0.0  + 2.6.32.13-gf6fe658"). I'm not sure if it's the right
>> solution though - should we expect a call to netback_uevent before the
>> vif is properly set up? All I'm doing is returning 0 (success?) if the
>> drvdata hasn't been set yet.
>
> We've seen this just now too (with our non-pv-ops kernel). Since this
> can be called due to sysfs reads (starting in 2.6.22), the function
> must be prepared to be called at any time. I do think, however, that
> it should provide as much data as possible in this state, i.e. not
> plainly return zero in that case, but at least add the "script=" setting
> (which is independent of "be" being NULL).
>
> Even then we still depend on uevent not caching the output (but
> rather re-issuing the read) once the backend for the new vif is fully
> set up.
>
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

I've had the same issue using novell 2.6.32 dom0 kernel, once portage
had upgraded udev to 154 and udev had been restarted I tried creating
a domain and it triggered null pointer dereference bug:


May 27 13:40:58 ubermicro kernel: BUG: unable to handle kernel NULL
pointer dereference at 0000000000000008
May 27 13:40:58 ubermicro kernel: IP: [<ffffffff80474f7d>]
netback_uevent+0x1d/0x96
May 27 13:40:58 ubermicro kernel: PGD 39e76d067 PUD 39ae8b067 PMD 0
May 27 13:40:58 ubermicro kernel: Oops: 0000 [#1] SMP
May 27 13:40:58 ubermicro kernel: last sysfs file:
/sys/devices/xen-backend/vif-1-0/uevent
May 27 13:40:58 ubermicro kernel: CPU 1
May 27 13:40:58 ubermicro kernel: Modules linked in: ipv6 coretemp
w83627hf w83793 hwmon_vid bnep rfcomm l2cap crc16 xen_scsibk st
snd_hda_codec_realtek snd_hda_intel snd_hda_codec nvidia(P) sym53c8xx
igb snd_usb_audio btusb i2c_i801 snd_pcm i5k_amb iTCO_wdt
iTCO_vendor_support bluetooth snd_timer snd_page_alloc snd_usb_lib
snd_rawmidi snd_hwdep snd [last unloaded: microcode]
May 27 13:40:58 ubermicro kernel: Pid: 9691, comm: udevd Tainted: P
       2.6.32-xen-r2 #1 X7DWA
May 27 13:40:58 ubermicro kernel: RIP: e030:[<ffffffff80474f7d>]
[<ffffffff80474f7d>] netback_uevent+0x1d/0x96
May 27 13:40:58 ubermicro kernel: RSP: e02b:ffff8803a2333e18  EFLAGS: 00010246
May 27 13:40:58 ubermicro kernel: RAX: 0000000000000000 RBX:
ffff8803820d0e48 RCX: 0000000000000000
May 27 13:40:58 ubermicro kernel: RDX: ffff88038229213b RSI:
ffff880382292000 RDI: 0000000000000000
May 27 13:40:58 ubermicro kernel: RBP: ffff8803820d0e00 R08:
ffff880382292153 R09: 0000000000000002
May 27 13:40:58 ubermicro kernel: R10: 000000000000000a R11:
ffffffff80474f60 R12: ffff880382292000
May 27 13:40:58 ubermicro kernel: R13: ffff880382292000 R14:
ffff88039af51000 R15: ffff8803a559f7e0
May 27 13:40:58 ubermicro kernel: FS:  00007f337e1986f0(0000)
GS:ffff880001025000(0000) knlGS:0000000000000000
May 27 13:40:58 ubermicro kernel: CS:  e033 DS: 0000 ES: 0000 CR0:
0000000080050033
May 27 13:40:58 ubermicro kernel: CR2: 0000000000000008 CR3:
00000003a4a70000 CR4: 0000000000002660
May 27 13:40:58 ubermicro kernel: DR0: 0000000000000000 DR1:
0000000000000000 DR2: 0000000000000000
May 27 13:40:58 ubermicro kernel: DR3: 0000000000000000 DR6:
00000000ffff0ff0 DR7: 0000000000000400
May 27 13:40:58 ubermicro udevd[9366]: worker [9691] unexpectedly
returned with status 0x0009
May 27 13:40:58 ubermicro udevd[9366]: worker [9691] failed while
handling '/devices/xen-backend/vif-1-0'
May 27 13:40:58 ubermicro kernel: Process udevd (pid: 9691, threadinfo
ffff8803a2332000, task ffff8803e92b4050)
May 27 13:40:58 ubermicro kernel: Stack:
May 27 13:40:58 ubermicro kernel: ffff880382292000 ffff8803820d0e48
ffff880382292000 ffff88039e7b9460
May 27 13:40:58 ubermicro kernel: <0> ffff880382292000
ffffffff804518b1 ffff88039e1bcc00 ffff8803e9953b40
May 27 13:40:58 ubermicro kernel: <0> ffff8803820d0e58
0000000000000000 ffff8803820d0e58 ffffffff804519f6
May 27 13:40:58 ubermicro kernel: Call Trace:
May 27 13:40:58 ubermicro kernel: [<ffffffff804518b1>] ? dev_uevent+0x104/0x146
May 27 13:40:58 ubermicro kernel: [<ffffffff804519f6>] ? show_uevent+0x87/0xdb
May 27 13:40:58 ubermicro kernel: [<ffffffff80451595>] ? dev_attr_show+0x1f/0x42
May 27 13:40:58 ubermicro kernel: [<ffffffff802d6e82>] ?
sysfs_read_file+0xa7/0x125
May 27 13:40:58 ubermicro kernel: [<ffffffff8028cf7f>] ? vfs_read+0xaa/0x166
May 27 13:40:58 ubermicro kernel: [<ffffffff8028d0f7>] ? sys_read+0x45/0x6e
May 27 13:40:58 ubermicro kernel: [<ffffffff802093f8>] ?
system_call_fastpath+0x16/0x1b
May 27 13:40:58 ubermicro kernel: [<ffffffff80209390>] ? system_call+0x0/0x52
May 27 13:40:58 ubermicro kernel: Code: 45 6c 02 0f 85 bd fb ff ff e9
dd fb ff ff 41 55 41 54 49 89 f4 55 48 89 fd 53 48 8d 7f 48 48 83 ec
08 e8 f4 ed fd ff 31 c9 31 ff <4c> 8b 68 08 48 8b 75 08 48 c7 c2 0c bf
6f 80 e8 1c 41 ff ff 48
May 27 13:40:58 ubermicro kernel: RIP  [<ffffffff80474f7d>]
netback_uevent+0x1d/0x96
May 27 13:40:58 ubermicro kernel: RSP <ffff8803a2333e18>
May 27 13:40:58 ubermicro kernel: CR2: 0000000000000008
May 27 13:40:58 ubermicro kernel: ---[ end trace 182c8c81ec5f1320 ]---


| can confirm that downgrading udev to 151 fixed the problem.

Andy