Bug 629638

Created attachment 442646 [details]
proposed patch

Description of problem:
From kernel version 2.6.18-194.6.1 and above, when changing the /proc/sys/net/ipv4/conf/eth*/forwarding, the kernel crashes.

Version-Release number of selected component (if applicable):
kernel-2.6.18-194.6.1 and above (the bug is still present in 2.6.18-194.14).

How reproducible:
Easily. Install a kernel, and change the /proc/.../eth*/forwarding by writing something there. For example,
echo 1 > /proc/sys/net/ipv4/conf/eth0/forwarding


Steps to Reproduce:
1. install kernel version from 2.6.18-194.6.1 to 2.6.18-194.14 on a machine with at least one ethernet interface.
2. boot
3. echo 1 > /proc/sys/net/ipv4/conf/eth0/forwarding
  
Actual results:
The kernel oopses.

Expected results:
The kernel doesn't oops, and the value changes.

Additional info:

The backtrace:
Unable to handle kernel paging request at ffffffffffffff40 RIP:
[<ffffffff802618f7>] devinet_sysctl_forward+0x50/0x74
PGD 203067 PUD a371067 PMD 0
Oops: 0000 [1] SMP
last sysfs file: /devices/pci0000:00/0000:00:00.0/irq
CPU 0
Modules linked in: ipv6 xfrm_nalgo md5 nfs lockd fscache nfs_acl rpcsec_gss_krb5 auth_rpcgss testmgr_cipher testmgr aead crypto_blkcipher crypto_algapi crypto_api des sunrpc 8021q ip_conntrack_netbios_ns iptable_mangle ipt_MASQUERADE iptable_nat ip_nat xt_tcpudp xt_state xt_CONNMARK ip_conntrack nfnetlink xt_multiport iptable_filter ip_tables x_tables dm_multipath scsi_dh video backlight sbs power_meter hwmon i2c_ec dell_wmi wmi button battery asus_acpi acpi_memhotplug ac parport_pc lp parport joydev i2c_i801 i2c_core pcspkr serio_raw tg3 sg e752x_edac e1000e edac_mc dm_raid45 dm_message dm_region_hash dm_mem_cache dm_snapshot dm_zero dm_mirror dm_log dm_mod ata_piix libata shpchp mptspi mptscsih mptbase scsi_transport_spi sd_mod scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd
Pid: 3814, comm: bash Not tainted 2.6.18-194.11.1.el5 #1
RIP: 0010:[<ffffffff802618f7>]  [<ffffffff802618f7>] devinet_sysctl_forward+0x50/0x74
RSP: 0018:ffff8100673fbe88  EFLAGS: 00010202
RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffff8100673fbf50
RDX: 0000000000000000 RSI: ffff8100673fbe18 RDI: ffffffff803500c0
RBP: ffff81007f87e2e4 R08: ffff8100673fa000 R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000001 R12: ffff810076a6a008
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000002
FS:  00002b7e13f61dd0(0000) GS:ffffffff803ca000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffffffffffffff40 CR3: 0000000066aa5000 CR4: 00000000000006e0
Process bash (pid: 3814, threadinfo ffff8100673fa000, task ffff81006a888820)
Stack:  0000000000000002 ffff810076a6a008 ffff81007bda8580 ffff810071f6fa80
0000000000000001 ffffffff80095e1e 00000002b7e1774c ffff8100673fbf50
00002b7e1774c000 00002b7e1774c000 0000000000000002 ffff810071f6fa80
Call Trace:
[<ffffffff80095e1e>] do_rw_proc+0xcb/0x126
[<ffffffff80016a17>] vfs_write+0xce/0x174
[<ffffffff800172e4>] sys_write+0x45/0x6e
[<ffffffff8005d28d>] tracesys+0xd5/0xe0

Code: 48 8b b8 40 ff ff ff e8 ff ef fc ff e8 d7 4e fd ff 31 ff e8
RIP  [<ffffffff802618f7>] devinet_sysctl_forward+0x50/0x74
RSP <ffff8100673fbe88>

The patch that introduced it:
http://patchwork.usersys.redhat.com/patch/24700/ (backporting from the upstream the devinet_sysctl_forward) - bz https://bugzilla.redhat.com/show_bug.cgi?id=582367#c14 .

The bug appears because the ->extra1 field isn't initialized properly. It should contain the pointer to the correct ipv4_devconf structure, so that we could get the actual device from that pointer:

1199 static int devinet_sysctl_forward(ctl_table *ctl, int write,
1200                   struct file* filp, void __user *buffer,
1201                   size_t *lenp, loff_t *ppos)
1202 {   
1203     int *valp = ctl->data;
1204     int val = *valp;
1205     int ret = proc_dointvec(ctl, write, filp, buffer, lenp, ppos);
1206 
1207     if (write && *valp != val) {
1208         if (valp != &ipv4_devconf_dflt.forwarding) {
1209             rtnl_lock();             
1210             if (valp == &ipv4_devconf.forwarding) {
1211                 inet_forward_change();
1212             } else if (*valp) {
1213                 struct ipv4_devconf *cnf = ctl->extra1;
1214                 struct in_device *idev =
1215                     container_of(cnf, struct in_device, cnf);
1216                 dev_disable_lro(idev->dev);
1217             }
1218             rtnl_unlock();
1219             rt_cache_flush(0);
1220         }
1221     }


But ->extra1 isn't initialized. The initialization is made in devinet_sysctl_register:

1539 static void devinet_sysctl_register(struct in_device *in_dev,
1540                     struct ipv4_devconf *p)
1541 {
...
1550     for (i = 0; i < ARRAY_SIZE(t->devinet_vars) - 1; i++) {
1551 
1552         if (check_ext_conf(&t->devinet_vars[i], in_dev,
1553                    p == &ipv4_devconf_dflt))
1554             continue;
1555 
1556         t->devinet_vars[i].data += (char *)p - (char *)&ipv4_devconf;
1557         t->devinet_vars[i].de = NULL;
1558     }

and only the .data is initialized, extra1 is not. extra1 should contain the p pointer to ipv4_devconf.

Proposed patch attached.