* NULL pointer dereference bug when unbinding 'igb' -driver from Intel i210 ethernet card
@ 2020-01-08 15:38 Antti Laakso
2020-01-13 13:00 ` [PATCH] ptp: free ptp device pin descriptors properly Vladis Dronov
0 siblings, 1 reply; 4+ messages in thread
From: Antti Laakso @ 2020-01-08 15:38 UTC (permalink / raw)
To: vdronov
Cc: netdev, sjohnsto, vlovejoy, richardcochran, davem, artem.bityutskiy
Hello,
Unbinding Intel 'igb' driver from Intel I210 PCIe ethernet card causes crash on kernel 5.5-rc5.
Running git bisect pointed to following patch triggering the issue:
commit a33121e5487b424339636b25c35d3a180eaa5f5e
Author: Vladis Dronov <vdronov@redhat.com>
Date: Fri Dec 27 03:26:27 2019 +0100
ptp: fix the race between the release of ptp_clock and cdev
Steps to reproduce:
$ lspci -D | grep I210
0000:18:00.0 Ethernet controller: Intel Corporation I210 Gigabit Network Connection (rev 03)
$ ls -l /sys/class/net/i210/device/driver/
total 0
lrwxrwxrwx 1 root root 0 Jan 8 15:59 0000:18:00.0 ->
../../../../devices/pci0000:17/0000:17:00.0/0000:18:00.0
--w------- 1 root root 4096 Jan 8 15:59 bind
lrwxrwxrwx 1 root root 0 Jan 8 15:59 module -> ../../../../module/igb
--w------- 1 root root 4096 Jan 8 15:59 new_id
--w------- 1 root root 4096 Jan 8 15:59 remove_id
--w------- 1 root root 4096 Jan 8 15:56 uevent
--w------- 1 root root 4096 Jan 8 15:59 unbind
$ echo "0000:18:00.0" > /sys/bus/pci/drivers/igb/unbind
[ 878.869098] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 878.876060] #PF: supervisor read access in kernel mode
[ 878.881197] #PF: error_code(0x0000) - not-present page
[ 878.886335] PGD 0 P4D 0
[ 878.888876] Oops: 0000 [#1] SMP NOPTI
[ 878.892541] CPU: 22 PID: 1844 Comm: bash Not tainted 5.5.0-rc5 #14
[ 878.898718] Hardware name: Intel Corporation S2600WFS/S2600WFS, BIOS
SE5C620.86B.0D.01.0395.022720191340 02/27/2019
[ 878.909151] RIP: 0010:strlen+0x0/0x20
[ 878.912818] Code: f6 82 00 fd ee a4 20 74 14 48 c7 c1 00 fd ee a4 48 83 c0 01 0f b6 10 f6 04 11
20 75 f3 c3 66 66 2e 0f 1f 84 00 00 00 00 00 90 <80> 3f 00 74 10 48 89 f8 48 83 c0 01 80 38 00 75 f7
48 29 f8 c3 31
[ 878.931565] RSP: 0018:ffff9f898f9b3c90 EFLAGS: 00010246
[ 878.936789] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[ 878.943921] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 878.951052] RBP: 0000000000000000 R08: 0000000000000000 R09: ffff91e8f6758898
[ 878.958183] R10: 0000000000000000 R11: ffff91e8f6758380 R12: 0000000000000000
[ 878.965309] R13: 0000000000000000 R14: ffff9f898f9b3f08 R15: ffff91e9175127a0
[ 878.972439] FS: 00007f00a0020740(0000) GS:ffff91e920b80000(0000) knlGS:0000000000000000
[ 878.980525] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 878.986272] CR2: 0000000000000000 CR3: 00000017b4c10003 CR4: 00000000007606e0
[ 878.993404] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 879.000536] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 879.007666] PKRU: 55555554
[ 879.010369] Call Trace:
[ 879.012819] kernfs_name_hash+0x12/0x80
[ 879.016654] kernfs_find_ns+0x35/0xc0
[ 879.020321] kernfs_remove_by_name_ns+0x31/0x90
[ 879.024855] remove_files.isra.1+0x30/0x70
[ 879.028953] sysfs_remove_group+0x3d/0x80
[ 879.032964] sysfs_remove_groups+0x29/0x40
[ 879.037064] device_remove_attrs+0x39/0x70
[ 879.041163] device_del+0x169/0x3f0
[ 879.044657] cdev_device_del+0x15/0x30
[ 879.048412] posix_clock_unregister+0x21/0x50
[ 879.052769] ptp_clock_unregister+0x6e/0x80 [ptp]
[ 879.057479] igb_ptp_stop+0x21/0x50 [igb]
[ 879.061493] igb_remove+0x46/0x120 [igb]
[ 879.065420] pci_device_remove+0x3b/0xc0
[ 879.069344] device_release_driver_internal+0xe5/0x1c0
[ 879.074483] unbind_store+0x94/0x120
[ 879.078063] kernfs_fop_write+0xc1/0x1a0
[ 879.081990] vfs_write+0xa5/0x1a0
[ 879.085305] ksys_write+0x59/0xd0
[ 879.088627] do_syscall_64+0x55/0x1d0
[ 879.092292] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 879.097343] RIP: 0033:0x7f00a01154b7
[ 879.100923] Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04
25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24
18 48 89 74 24
[ 879.119668] RSP: 002b:00007ffcd6953b18 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 879.127233] RAX: ffffffffffffffda RBX: 000000000000000d RCX: 00007f00a01154b7
[ 879.134363] RDX: 000000000000000d RSI: 000055a7075c9f60 RDI: 0000000000000001
[ 879.141494] RBP: 000055a7075c9f60 R08: 000000000000000a R09: 000000000000000c
[ 879.148619] R10: 000055a707503c10 R11: 0000000000000246 R12: 000000000000000d
[ 879.155752] R13: 00007f00a01e6500 R14: 000000000000000d R15: 00007f00a01e6700
[ 879.162883] Modules linked in: intel_rapl_msr intel_rapl_common skx_edac iTCO_wdt
x86_pkg_temp_thermal iTCO_vendor_support watchdog ipmi_ssif kvm lpc_ich ipmi_si irqbypass pcspkr
ioatdma mfd_core i2c_i801 ipmi_devintf wmi ipmi_msghandler acpi_pad vfat fat dm_multipath
ixgbe igb i40e dca mdio nvme megaraid_sas crct10dif_pclmul ptp crc32_pclmul nvme_core crc32c_intel
pps_core i2c_algo_bit sunrpc
[ 879.197670] CR2: 0000000000000000
[ 879.200990] ---[ end trace c61f33a12ae52fc5 ]---
[ 879.298587] RIP: 0010:strlen+0x0/0x20
[ 879.302249] Code: f6 82 00 fd ee a4 20 74 14 48 c7 c1 00 fd ee a4 48 83 c0 01 0f b6 10 f6 04 11
20 75 f3 c3 66 66 2e 0f 1f 84 00 00 00 00 00 90 <80> 3f 00 74 10 48 89 f8 48 83 c0 01 80 38 00 75 f7
48 29 f8 c3 31
[ 879.320993] RSP: 0018:ffff9f898f9b3c90 EFLAGS: 00010246
[ 879.326221] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[ 879.333351] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 879.340483] RBP: 0000000000000000 R08: 0000000000000000 R09: ffff91e8f6758898
[ 879.347618] R10: 0000000000000000 R11: ffff91e8f6758380 R12: 0000000000000000
[ 879.354746] R13: 0000000000000000 R14: ffff9f898f9b3f08 R15: ffff91e9175127a0
[ 879.361872] FS: 00007f00a0020740(0000) GS:ffff91e920b80000(0000) knlGS:0000000000000000
[ 879.369957] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 879.375703] CR2: 0000000000000000 CR3: 00000017b4c10003 CR4: 00000000007606e0
[ 879.382833] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 879.389958] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 879.397089] PKRU: 55555554
Usually problem is hit when doing unbind first time.
But sometimes binding device back to 'igb'
-driver, and unbinding it again, is needed:
$ echo "0000:18:00.0" > /sys/bus/pci/drivers/igb/bind
$
echo "0000:18:00.0" > /sys/bus/pci/drivers/igb/unbind
At most 50 unbind/bind iterations is needed to reproduce the issue.
Reproduced on Intel Xeon 8260L, with Intel I210 1GbE PCIe Ethernet Card.
--
Antti Laakso
antti.laakso@intel.com
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] ptp: free ptp device pin descriptors properly
2020-01-08 15:38 NULL pointer dereference bug when unbinding 'igb' -driver from Intel i210 ethernet card Antti Laakso
@ 2020-01-13 13:00 ` Vladis Dronov
2020-01-14 4:26 ` Richard Cochran
2020-01-14 18:59 ` David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Vladis Dronov @ 2020-01-13 13:00 UTC (permalink / raw)
To: Antti Laakso, netdev
Cc: Richard Cochran, vdronov, sjohnsto, vlovejoy, linux-kernel,
davem, artem.bityutskiy
There is a bug in ptp_clock_unregister(), where ptp_cleanup_pin_groups()
first frees ptp->pin_{,dev_}attr, but then posix_clock_unregister() needs
them to destroy a related sysfs device.
These functions can not be just swapped, as posix_clock_unregister() frees
ptp which is needed in the ptp_cleanup_pin_groups(). Fix this by calling
ptp_cleanup_pin_groups() in ptp_clock_release(), right before ptp is freed.
This makes this patch fix an UAF bug in a patch which fixes an UAF bug.
Reported-by: Antti Laakso <antti.laakso@intel.com>
Fixes: a33121e5487b ("ptp: fix the race between the release of ptp_clock and cdev")
Link: https://lore.kernel.org/netdev/3d2bd09735dbdaf003585ca376b7c1e5b69a19bd.camel@intel.com/
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
---
drivers/ptp/ptp_clock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 61fafe0374ce..b84f16bbd6f2 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -170,6 +170,7 @@ static void ptp_clock_release(struct device *dev)
{
struct ptp_clock *ptp = container_of(dev, struct ptp_clock, dev);
+ ptp_cleanup_pin_groups(ptp);
mutex_destroy(&ptp->tsevq_mux);
mutex_destroy(&ptp->pincfg_mux);
ida_simple_remove(&ptp_clocks_map, ptp->index);
@@ -302,9 +303,8 @@ int ptp_clock_unregister(struct ptp_clock *ptp)
if (ptp->pps_source)
pps_unregister_source(ptp->pps_source);
- ptp_cleanup_pin_groups(ptp);
-
posix_clock_unregister(&ptp->clock);
+
return 0;
}
EXPORT_SYMBOL(ptp_clock_unregister);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ptp: free ptp device pin descriptors properly
2020-01-13 13:00 ` [PATCH] ptp: free ptp device pin descriptors properly Vladis Dronov
@ 2020-01-14 4:26 ` Richard Cochran
2020-01-14 18:59 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: Richard Cochran @ 2020-01-14 4:26 UTC (permalink / raw)
To: Vladis Dronov
Cc: Antti Laakso, netdev, sjohnsto, vlovejoy, linux-kernel, davem,
artem.bityutskiy
On Mon, Jan 13, 2020 at 02:00:09PM +0100, Vladis Dronov wrote:
> There is a bug in ptp_clock_unregister(), where ptp_cleanup_pin_groups()
> first frees ptp->pin_{,dev_}attr, but then posix_clock_unregister() needs
> them to destroy a related sysfs device.
>
> These functions can not be just swapped, as posix_clock_unregister() frees
> ptp which is needed in the ptp_cleanup_pin_groups(). Fix this by calling
> ptp_cleanup_pin_groups() in ptp_clock_release(), right before ptp is freed.
>
> This makes this patch fix an UAF bug in a patch which fixes an UAF bug.
>
> Reported-by: Antti Laakso <antti.laakso@intel.com>
> Fixes: a33121e5487b ("ptp: fix the race between the release of ptp_clock and cdev")
> Link: https://lore.kernel.org/netdev/3d2bd09735dbdaf003585ca376b7c1e5b69a19bd.camel@intel.com/
> Signed-off-by: Vladis Dronov <vdronov@redhat.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ptp: free ptp device pin descriptors properly
2020-01-13 13:00 ` [PATCH] ptp: free ptp device pin descriptors properly Vladis Dronov
2020-01-14 4:26 ` Richard Cochran
@ 2020-01-14 18:59 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2020-01-14 18:59 UTC (permalink / raw)
To: vdronov
Cc: antti.laakso, netdev, richardcochran, sjohnsto, vlovejoy,
linux-kernel, artem.bityutskiy
From: Vladis Dronov <vdronov@redhat.com>
Date: Mon, 13 Jan 2020 14:00:09 +0100
> There is a bug in ptp_clock_unregister(), where ptp_cleanup_pin_groups()
> first frees ptp->pin_{,dev_}attr, but then posix_clock_unregister() needs
> them to destroy a related sysfs device.
>
> These functions can not be just swapped, as posix_clock_unregister() frees
> ptp which is needed in the ptp_cleanup_pin_groups(). Fix this by calling
> ptp_cleanup_pin_groups() in ptp_clock_release(), right before ptp is freed.
>
> This makes this patch fix an UAF bug in a patch which fixes an UAF bug.
>
> Reported-by: Antti Laakso <antti.laakso@intel.com>
> Fixes: a33121e5487b ("ptp: fix the race between the release of ptp_clock and cdev")
> Link: https://lore.kernel.org/netdev/3d2bd09735dbdaf003585ca376b7c1e5b69a19bd.camel@intel.com/
> Signed-off-by: Vladis Dronov <vdronov@redhat.com>
Applied, thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-14 19:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-08 15:38 NULL pointer dereference bug when unbinding 'igb' -driver from Intel i210 ethernet card Antti Laakso
2020-01-13 13:00 ` [PATCH] ptp: free ptp device pin descriptors properly Vladis Dronov
2020-01-14 4:26 ` Richard Cochran
2020-01-14 18:59 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).