Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH net] net: phy: Warn if phy is attached when removing
To:     Jakub Kicinski <kuba@kernel.org>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>, netdev@vger.kernel.org,
        Paolo Abeni <pabeni@redhat.com>,
        "David S . Miller" <davem@davemloft.net>,
        linux-kernel@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
        Vladimir Oltean <olteanv@gmail.com>
References: <20220816163701.1578850-1-sean.anderson@seco.com>
 <20220818102443.4c7c50e8@kernel.org>
From:   Sean Anderson <sean.anderson@seco.com>
Message-ID: <ffc5114e-07c4-7154-b643-01dd86cbc4c2@seco.com>
Date:   Thu, 18 Aug 2022 13:32:36 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
In-Reply-To: <20220818102443.4c7c50e8@kernel.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TkZBSjg0UnB6d2gvdm8rekwyMGhpeERJSnpLdWNNeVh1bG5CTDVGKzNEM2Nt?=
 =?utf-8?B?MlU2OStRVDAzNXFtN0tmdDRCckFrR0dOVDdrZzZpSFBqMU5Fa3Jwa0c2WXND?=
 =?utf-8?B?ZXc5dXk0MGhSMmZwbSt4ZCt1MXpUUEg3RkN3aU9aRmpia0NWV1RtazNlTFBR?=
 =?utf-8?B?MzZxdDlNc3R5NXNhN1BtMEtldmIvVWU4SFNFYWhMRG1Ia2xGemF0ajNNUVk5?=
 =?utf-8?B?U2pKeE9nOWdKUW9CaVlXbXB3dk00dVJlQnhlaUp5Z3RXSExxT25DMGU3eTNo?=
 =?utf-8?B?Ymg0SUphajB5dUdpN014cHlBWGxycHppQytReTRDK0lmbmUyTllSYUtpVWFj?=
 =?utf-8?B?T0svTjRrRTd0MG13T2FWdHJQQnp3WEV6N2IwRDBwbGc4N0JJSTE5MGJxUkpC?=
 =?utf-8?B?N1hYOVRqcWhuTkxVdllDamYwT1p6M2F1Z2VHNjJ4NHJocnlzSTNiMmJPWmJo?=
 =?utf-8?B?L3UvM3RFei9DNllpNytBekx1N1VZWUFKTWVDUXlmVFlZcWN6ZENPMTJmY2Yr?=
 =?utf-8?B?Y0RhODlHdFhqZEhWWUhDRS9rRlJPc0dUZ1R3TTJJQlhvUGoweTVCbDlrN2lF?=
 =?utf-8?B?UmdsSkZmL2lQMGNmL0MwRWZUZFRKcSsxQXVJeFFLTnlkYUJDdUNMTjA2Wk1K?=
 =?utf-8?B?ZWh2bHNKSUdzNzI0MFUvUVFxWjZkeFdTNFVGS3hpd2k1SDZwSEc2UXU3bkVu?=
 =?utf-8?B?RW1aQ1JTSVZvTE1JZVBkZkE0azVUMklRTFZPc3NNU3hmREx5YUd1Q3pwSHNF?=
 =?utf-8?B?Y0RFMTczMG5zcHQ0UEIya25QcTQ4NnNjeUVsdW5Db1VDVnhRRlc2QjJ1eVAy?=
 =?utf-8?B?cWE3NElwSGJXRXo2dHJjY1Y1SSttQ2VIaVBGU21keVV4OXlSU2tqa3h5Tlgw?=
 =?utf-8?B?ZVJpR0xyaW1FUmoxQ3o2SlYvbHFnTHNXZnh0eGc1eWljalhNam9wNUJEbmxv?=
 =?utf-8?B?bmpyUzFLVkxIQkhlVGdGcUw5eXg3NlBFaDVZdjVPbkYxK1pJZ2lwWDFqNW4y?=
 =?utf-8?B?aFRiVlkzSkJmOVM1Q04xODFXeU1zNmt3NjRBWHRMRDRiSXpGamUrWDFQQk05?=
 =?utf-8?B?eHoyTFZqQWloM0s5elhVRExtbVFTeFJGeEgrb0FQWHpBcCt0Tyt4NUZPWE1a?=
 =?utf-8?B?eXNzMjRjOE5SOUExYzNJOEdLTXRhN1F4Vkx5Vlp1MVlCMVBZTU81V21zbmFM?=
 =?utf-8?B?dHBmLytyKzZ5UlRqalBsVVhvL01GYnQvbE1XdUFuZkdCV29lVlV2NC8ycFph?=
 =?utf-8?B?aWZzNERtMEgvclRqMkJBMzNlZ3BjVkREcVBVSGs1em1ubUc3ZHlsSGhxUXo1?=
 =?utf-8?B?d0xZVDdLSFBlWEppWHJMVzVWR2k0blR6TWw4bWJQaE1jVms5UnljeEt4MmtE?=
 =?utf-8?B?bElydHNRRHg5MW51NStSME9qK2pweXg1SmdUZ1kwRDBoLzUxU1VTcVBOR0Ni?=
 =?utf-8?B?Zmt1M3pLTk1Lcm11QzM3Zno5Q1BuSGxuZUwxekpoRGFYenM2T01Cc1YwNjVR?=
 =?utf-8?B?a0F4MkZhK0Z4NmlFT3BlZjNYckJhbFZrci9zWm85aWEwSWg3WTRVV3FlSXVi?=
 =?utf-8?B?YkdkVGtFVEVWQVFWeDRXZk10cEpDNTZTMXQ4OTQ0MEk3TlNEYnBqRlBVWFV5?=
 =?utf-8?B?MkVva3Q2T3M5WEUvV2dUbjNwY1BYSmRoaUM1UFM2eVVKc09QTjM4QWRWSm5E?=
 =?utf-8?B?NTNtNkhCNVYzekwyQytpVTQ4Wm5zVjA2U0kwRHBqWDRhcVRIZm1vUnNVd2tZ?=
 =?utf-8?B?ZU1CaHQ3dmtTOXFJRlNiWUliMXZSTWFrWVdVZUpSZ09YWnczWjIvNjUvNVNt?=
 =?utf-8?B?Mm0xc3dxTWVPZVA4Yi9uMUpramxvNkhyclBwb0hTV1RKZ2wzWjJvTndqeDhs?=
 =?utf-8?B?dy9HdFZlNEN5ZzAyY24xbVF5cDhUTkJ5eU9TZE1tdllkVDA3bXlLd0w4K1RQ?=
 =?utf-8?B?dDNFakptdTduampEdzFDOUJrdml4TnczOXlCYnlzNnUxZElKSUpmTnNuZWx2?=
 =?utf-8?B?Tm14WWRHR0pnc2I0S3dGUk5HOTBvTnNVaHErd2NhKzcxUS9FTE9JV2VoNWdN?=
 =?utf-8?B?TXo2aXpBWWdyajJVa2laT3haTk9jc2wrNDgrWTVqOFduTENhRFIxYjdBcWFm?=
 =?utf-8?B?V2F4NTdUY1J2a1FxcXBpd2NJZ2JOWjArc2oxZFJENVBiOFNyYzFFY0dkTXFD?=
 =?utf-8?B?Unc9PQ==?=
X-OriginatorOrg: seco.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 107ac566-da9d-4d36-ab90-08da813fa682
X-MS-Exchange-CrossTenant-AuthSource: DB7PR03MB4972.eurprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Aug 2022 17:32:40.3788
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bebe97c3-6438-442e-ade3-ff17aa50e733
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ji23bOuJ6NXr4m2gdsGZ1rGE9va53mQOE/lCGQxHsENG6/zVW94vGBnEcECO8ITjhYwv8i2qXqTCszB3toP98A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0301MB2495
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 8/18/22 1:24 PM, Jakub Kicinski wrote:
> On Tue, 16 Aug 2022 12:37:01 -0400 Sean Anderson wrote:
>> netdevs using phylib can be oopsed from userspace in the following
>> manner:
>> 
>> $ ip link set $iface up
>> $ echo $(basename $(readlink /sys/class/net/$iface/phydev)) > \
>>       /sys/class/net/$iface/phydev/driver/unbind
>> $ ip link set $iface down
>> 
>> However, the traceback provided is a bit too late, since it does not
>> capture the root of the problem (unbinding the driver). It's also
>> possible that the memory has been reallocated if sufficient time passes
>> between when the phy is detached and when the netdev touches the phy
>> (which could result in silent memory corruption). Add a warning at the
>> source of the problem. A future patch could make this more robust by
>> calling dev_close.
> 
> Hm, so we're adding the warning to get more detailed reports "from the
> field"? Guess we've all done that, so fair.

My suspicion is that this case never occurs, since users don't expect to
be able to remove the phy while the interface is running (and so don't
attempt it). If we do end up getting reports of this bug, then we will
need to create a more robust fix. My intention is to take the same
strategy for PCS devices as whatever we do here, since the issue is
analogous.

--Sean