Since phy driver features became a link_mode bitmap, phy drivers that
don't have a list of features configured will cause the kernel to crash
when probed.
Prevent the phy driver from registering if the features field is missing.
Fixes: 719655a14971 ("net: phy: Replace phy driver features u32 with link_mode bitmap")
Reported-by: Scott Wood <[email protected]>
Signed-off-by: Camelia Groza <[email protected]>
---
This patch should be merged to 4.20 stable.
---
drivers/net/phy/phy_device.c | 3 +++
include/linux/phy.h | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 5199000..f70dfb3 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2243,6 +2243,9 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
{
int retval;
+ if (WARN_ON(!new_driver->features))
+ return -EINVAL;
+
new_driver->mdiodrv.flags |= MDIO_DEVICE_IS_PHY;
new_driver->mdiodrv.driver.name = new_driver->name;
new_driver->mdiodrv.driver.bus = &mdio_bus_type;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3b051f7..ec3b6d5 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -467,8 +467,8 @@ struct phy_device {
* only works for PHYs with IDs which match this field
* name: The friendly name of this PHY type
* phy_id_mask: Defines the important bits of the phy_id
- * features: A list of features (speed, duplex, etc) supported
- * by this PHY
+ * features: A mandatory list of features (speed, duplex, etc)
+ * supported by this PHY
* flags: A bitfield defining certain other features this PHY
* supports (like interrupts)
*
--
1.9.1
On Tue, Jan 15, 2019 at 02:35:19PM +0200, Camelia Groza wrote:
> Since phy driver features became a link_mode bitmap, phy drivers that
> don't have a list of features configured will cause the kernel to crash
> when probed.
>
> Prevent the phy driver from registering if the features field is missing.
>
> Fixes: 719655a14971 ("net: phy: Replace phy driver features u32 with link_mode bitmap")
> Reported-by: Scott Wood <[email protected]>
> Signed-off-by: Camelia Groza <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
FYI, we noticed the following commit (built with gcc-7):
commit: b8154ef682a9a745880ecbb2ee26b16297c9bfd0 ("[PATCH net] net: phy: phy driver features are mandatory")
url: https://github.com/0day-ci/linux/commits/Camelia-Groza/net-phy-phy-driver-features-are-mandatory/20190116-004308
in testcase: trinity
with following parameters:
runtime: 300s
test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 512M
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+--------------------------------------------------------------+------------+------------+
| | 2f960bd056 | b8154ef682 |
+--------------------------------------------------------------+------------+------------+
| boot_successes | 45 | 0 |
| boot_failures | 1 | 4 |
| BUG:kernel_timeout_in_torture_test_stage | 1 | |
| WARNING:at_drivers/net/phy/phy_device.c:#phy_driver_register | 0 | 4 |
| EIP:phy_driver_register | 0 | 4 |
+--------------------------------------------------------------+------------+------------+
[ 458.251512] WARNING: CPU: 0 PID: 1 at drivers/net/phy/phy_device.c:2246 phy_driver_register+0x90/0xe0
[ 458.260864] CPU: 0 PID: 1 Comm: swapper Tainted: G T 4.20.0-11080-gb8154ef6 #158
[ 458.260864] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[ 458.260864] EIP: phy_driver_register+0x90/0xe0
[ 458.260864] Code: 5d 89 7b 08 89 03 89 d8 c7 43 14 02 00 00 00 e8 06 d3 e1 ff 85 c0 89 c6 75 39 83 c4 0c 89 f0 5b 5e 5f 5d c3 8d b6 00 00 00 00 <0f> 0b b8 38 5b 2e 5e c7 04 24 00 00 00 00 31 c9 ba 01 00 00 00 be
[ 458.260864] EAX: 5e2e5b50 EBX: 5e1a8fe0 ECX: 00000000 EDX: 00000001
[ 458.260864] ESI: 00000000 EDI: 00000000 EBP: 4010defc ESP: 4010dee4
[ 458.260864] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00210246
[ 458.260864] CR0: 80050033 CR2: 00000000 CR3: 1e43a000 CR4: 000006b0
[ 458.260864] Call Trace:
[ 458.260864] ? phy_drivers_register+0x37/0x90
[ 458.260864] ? phy_module_init+0x1b/0x1b
[ 458.260864] ? phy_module_init+0x19/0x1b
[ 458.260864] ? do_one_initcall+0xd9/0x24b
[ 458.260864] ? rcu_read_lock_sched_held+0x3f/0x60
[ 458.260864] ? trace_initcall_level+0x90/0xc3
[ 458.260864] ? kernel_init_freeable+0xe1/0x181
[ 458.260864] ? kernel_init_freeable+0x106/0x181
[ 458.260864] ? rest_init+0x140/0x140
[ 458.260864] ? kernel_init+0x10/0x100
[ 458.260864] ? schedule_tail_wrapper+0x9/0xc
[ 458.260864] ? ret_from_fork+0x19/0x24
[ 458.260864] irq event stamp: 1825870
[ 458.260864] hardirqs last enabled at (1825869): [<5c7633e6>] free_unref_page+0x56/0x70
[ 458.260864] hardirqs last disabled at (1825870): [<5c6013f1>] trace_hardirqs_off_thunk+0xc/0x1b
[ 458.260864] softirqs last enabled at (1825822): [<5d8c0934>] __do_softirq+0x2d4/0x483
[ 458.260864] softirqs last disabled at (1825817): [<5c60d8d1>] do_softirq_own_stack+0x21/0x30
[ 458.260864] random: get_random_bytes called from print_oops_end_marker+0x57/0x70 with crng_init=0
[ 458.260864] ---[ end trace 370e1ec5b20c08ee ]---
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks,
Rong Chen
On Wed, Jan 16, 2019 at 04:58:58PM +0800, kernel test robot wrote:
> FYI, we noticed the following commit (built with gcc-7):
>
> commit: b8154ef682a9a745880ecbb2ee26b16297c9bfd0 ("[PATCH net] net: phy: phy driver features are mandatory")
> url: https://github.com/0day-ci/linux/commits/Camelia-Groza/net-phy-phy-driver-features-are-mandatory/20190116-004308
>
>
> in testcase: trinity
> with following parameters:
>
> runtime: 300s
>
> test-description: Trinity is a linux system call fuzz tester.
> test-url: http://codemonkey.org.uk/projects/trinity/
>
>
> on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 512M
>
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
>
>
> +--------------------------------------------------------------+------------+------------+
> | | 2f960bd056 | b8154ef682 |
> +--------------------------------------------------------------+------------+------------+
> | boot_successes | 45 | 0 |
> | boot_failures | 1 | 4 |
> | BUG:kernel_timeout_in_torture_test_stage | 1 | |
> | WARNING:at_drivers/net/phy/phy_device.c:#phy_driver_register | 0 | 4 |
> | EIP:phy_driver_register | 0 | 4 |
> +--------------------------------------------------------------+------------+------------+
>
>
>
> [ 458.251512] WARNING: CPU: 0 PID: 1 at drivers/net/phy/phy_device.c:2246 phy_driver_register+0x90/0xe0
> [ 458.260864] CPU: 0 PID: 1 Comm: swapper Tainted: G T 4.20.0-11080-gb8154ef6 #158
> [ 458.260864] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> [ 458.260864] EIP: phy_driver_register+0x90/0xe0
> [ 458.260864] Code: 5d 89 7b 08 89 03 89 d8 c7 43 14 02 00 00 00 e8 06 d3 e1 ff 85 c0 89 c6 75 39 83 c4 0c 89 f0 5b 5e 5f 5d c3 8d b6 00 00 00 00 <0f> 0b b8 38 5b 2e 5e c7 04 24 00 00 00 00 31 c9 ba 01 00 00 00 be
> [ 458.260864] EAX: 5e2e5b50 EBX: 5e1a8fe0 ECX: 00000000 EDX: 00000001
> [ 458.260864] ESI: 00000000 EDI: 00000000 EBP: 4010defc ESP: 4010dee4
> [ 458.260864] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00210246
> [ 458.260864] CR0: 80050033 CR2: 00000000 CR3: 1e43a000 CR4: 000006b0
> [ 458.260864] Call Trace:
> [ 458.260864] ? phy_drivers_register+0x37/0x90
> [ 458.260864] ? phy_module_init+0x1b/0x1b
> [ 458.260864] ? phy_module_init+0x19/0x1b
> [ 458.260864] ? do_one_initcall+0xd9/0x24b
Hi Camelia
I think we need to extend your patch and print the name of the PHY
which is missing the features pointer. I don't see anything here which
allows me to determine which PHY caused the warning.
Thanks
Andrew
> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: Wednesday, January 16, 2019 15:41
> To: kernel test robot <[email protected]>
> Cc: Camelia Alexandra Groza <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [net] b8154ef682:
> WARNING:at_drivers/net/phy/phy_device.c:#phy_driver_register
>
> On Wed, Jan 16, 2019 at 04:58:58PM +0800, kernel test robot wrote:
> > FYI, we noticed the following commit (built with gcc-7):
> >
> > commit: b8154ef682a9a745880ecbb2ee26b16297c9bfd0 ("[PATCH net] net:
> phy: phy driver features are mandatory")
> > url: https://github.com/0day-ci/linux/commits/Camelia-Groza/net-phy-phy-driver-features-are-mandatory/20190116-004308
> >
> >
> > in testcase: trinity
> > with following parameters:
> >
> > runtime: 300s
> >
> > test-description: Trinity is a linux system call fuzz tester.
> > test-url: http://codemonkey.org.uk/projects/trinity/
> >
> >
> > on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2
> -m 512M
> >
> > caused below changes (please refer to attached dmesg/kmsg for entire
> log/backtrace):
> >
> >
> > +--------------------------------------------------------------+------------+------------
> +
> > | | 2f960bd056 | b8154ef682 |
> > +--------------------------------------------------------------+------------+------------
> +
> > | boot_successes | 45 | 0 |
> > | boot_failures | 1 | 4 |
> > | BUG:kernel_timeout_in_torture_test_stage | 1 | |
> > | WARNING:at_drivers/net/phy/phy_device.c:#phy_driver_register | 0
> | 4 |
> > | EIP:phy_driver_register | 0 | 4 |
> > +--------------------------------------------------------------+------------+------------
> +
> >
> >
> >
> > [ 458.251512] WARNING: CPU: 0 PID: 1 at
> drivers/net/phy/phy_device.c:2246 phy_driver_register+0x90/0xe0
> > [ 458.260864] CPU: 0 PID: 1 Comm: swapper Tainted: G T 4.20.0-
> 11080-gb8154ef6 #158
> > [ 458.260864] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.10.2-1 04/01/2014
> > [ 458.260864] EIP: phy_driver_register+0x90/0xe0
> > [ 458.260864] Code: 5d 89 7b 08 89 03 89 d8 c7 43 14 02 00 00 00 e8 06 d3 e1
> ff 85 c0 89 c6 75 39 83 c4 0c 89 f0 5b 5e 5f 5d c3 8d b6 00 00 00 00 <0f> 0b b8 38
> 5b 2e 5e c7 04 24 00 00 00 00 31 c9 ba 01 00 00 00 be
> > [ 458.260864] EAX: 5e2e5b50 EBX: 5e1a8fe0 ECX: 00000000 EDX: 00000001
> > [ 458.260864] ESI: 00000000 EDI: 00000000 EBP: 4010defc ESP: 4010dee4
> > [ 458.260864] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS:
> 00210246
> > [ 458.260864] CR0: 80050033 CR2: 00000000 CR3: 1e43a000 CR4: 000006b0
> > [ 458.260864] Call Trace:
> > [ 458.260864] ? phy_drivers_register+0x37/0x90
> > [ 458.260864] ? phy_module_init+0x1b/0x1b
> > [ 458.260864] ? phy_module_init+0x19/0x1b
> > [ 458.260864] ? do_one_initcall+0xd9/0x24b
>
> Hi Camelia
>
> I think we need to extend your patch and print the name of the PHY
> which is missing the features pointer. I don't see anything here which
> allows me to determine which PHY caused the warning.
Hi
Sure, I'll send a v2.
Camelia