From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5E8C6C4345F for ; Thu, 25 Apr 2024 18:46:34 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 0C114410C6; Thu, 25 Apr 2024 18:46:34 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id T_8zFiPSoOWB; Thu, 25 Apr 2024 18:46:31 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.166.34; helo=ash.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org B547B409F9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org; s=default; t=1714070791; bh=oHiZ5Zn9tsCqNJ1BNE+RN3qR9v/z0I5MXTQ5Y5xO2vY=; h=From:To:In-Reply-To:References:Date:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: Cc:From; b=uiEGdU4QfMPq5ZIjtWNxUxC2L4DLpr/XcWjQb0dVtZuBX33PVbdMGtr1mUYG0ao89 TIdPr/2q2m9CkZR8muGvJnhM2KB0XISfIs+FIfIHNY/iMGjac6YECe9+GtG5Dhkwu1 siy5sEvnzdjoCng94adZi9Cb5mdK0+y9XPFwZojQ1Lzyb3B+pZ+kbkA8QhSPftTcAQ sx1prbU9XKw0CosgtCTpYjEUK0xqal9w9sqYE/hQ1gUSz7Wwr4yXMScWuJAIMy8/5a OOLtEZb4B+rJwuTLeWGbIVGBPJyJ6xMY2XJ1QaUjAwQxUo+5aPaiQPwKog/BBqCbVk SvZEs2snpPIxg== Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by smtp4.osuosl.org (Postfix) with ESMTP id B547B409F9; Thu, 25 Apr 2024 18:46:31 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by ash.osuosl.org (Postfix) with ESMTP id BC1A41BF3C2 for ; Thu, 25 Apr 2024 18:46:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id A7F6760676 for ; Thu, 25 Apr 2024 18:46:29 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id EEIdFWDSQcTn for ; Thu, 25 Apr 2024 18:46:29 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=192.198.163.12; helo=mgamail.intel.com; envelope-from=vinicius.gomes@intel.com; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp3.osuosl.org E725760645 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org E725760645 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) by smtp3.osuosl.org (Postfix) with ESMTPS id E725760645 for ; Thu, 25 Apr 2024 18:46:28 +0000 (UTC) X-CSE-ConnectionGUID: 0JKzeZztQOGEOsXuYbcqbg== X-CSE-MsgGUID: Qe21O0OkT8WC12qmUNKSww== X-IronPort-AV: E=McAfee;i="6600,9927,11055"; a="13569737" X-IronPort-AV: E=Sophos;i="6.07,230,1708416000"; d="scan'208";a="13569737" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Apr 2024 11:46:27 -0700 X-CSE-ConnectionGUID: pkPPmTyARQKBAQElP1o99A== X-CSE-MsgGUID: itgeq33yQP+DFUv66C/KQQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,230,1708416000"; d="scan'208";a="25666087" Received: from unknown (HELO vcostago-mobl3) ([10.124.220.196]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Apr 2024 11:46:27 -0700 From: Vinicius Costa Gomes To: Corinna Vinschen In-Reply-To: References: <20240423102455.901469-1-vinschen@redhat.com> <033cce07-fe8f-42e6-8c27-7afee87fe13c@lunn.ch> <8734raxq4z.fsf@intel.com> Date: Thu, 25 Apr 2024 11:46:26 -0700 Message-ID: <87r0etwa9p.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1714070789; x=1745606789; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=ZsDqYu4ojuh9VuiJbaNfZoanFnnda5qja2yT5h3wsto=; b=ACC6a8sYWUGUej2p+Xo+Fa7Ql0P0AUWVPs/OFKwY5+EQwuMhwkYE4de9 9P1Dtfn0d7K2vcBwR9RhAoFQZxk/+Qwyt1wHn0DPGw14FdJ0LUFSqyvg/ 8Xd1O+Y6oT1Rd6WWjVlb/NaKFRBN6PBl+IP/BDOe5CbqFTnidfz1XK94a gPZY6/Btzb4d7yA02Ku3541cvoaGvxvmjetm27SlmxiHyRA3Dm6pDOyIW N5+FHVgNu/uYmFzeO+ylx6CBwSFf3ZQcKoskc5lArShDHbiroFIjRA8KQ GM225AWen+CCgcK475TXpYSYqRPfOl7U6pvskGe2AC8mt81zxVEVSouzB w==; X-Mailman-Original-Authentication-Results: smtp3.osuosl.org; dmarc=pass (p=none dis=none) header.from=intel.com X-Mailman-Original-Authentication-Results: smtp3.osuosl.org; dkim=pass (2048-bit key, unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=ACC6a8sY Subject: Re: [Intel-wired-lan] [PATCH] igc: fix a log entry using uninitialized netdev X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Lunn , intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" Corinna Vinschen writes: > On Apr 24 17:06, Vinicius Costa Gomes wrote: >> Andrew Lunn writes: >> >> > On Tue, Apr 23, 2024 at 12:24:54PM +0200, Corinna Vinschen wrote: >> >> During successful probe, igc logs this: >> >> >> >> [ 5.133667] igc 0000:01:00.0 (unnamed net_device) (uninitialized): PHC added >> >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> >> The reason is that igc_ptp_init() is called very early, even before >> >> register_netdev() has been called. So the netdev_info() call works >> >> on a partially uninitialized netdev. >> >> >> >> Fix this by calling igc_ptp_init() after register_netdev(), right >> >> after the media autosense check, just as in igb. Add a comment, >> >> just as in igb. >> > >> > The network stack can start sending and receiving packet before >> > register_netdev() returns. This is typical of NFS root for example. Is >> > there anything in igc_ptp_init() which could cause such packet >> > transfers to explode? >> > >> >> There might be a very narrow window (probably impossible?), what I can >> see is: >> >> 1. the netdevice is exposed to userspace; >> 2. userspace does the SIOCSHWTSTAMP ioctl() to enable TX timestamps; >> 3. userspace sends a packet that is going to be timestamped; >> >> if this happens before igc_ptp_init() is called, adapter->ptp_tx_lock is >> going to be uninitialized, and (3) is going to crash. > > The same would then be possible on igb as well, wouldn't it? > Given how many years igb is being used, perhaps "possible" is too strong :-) On igb what exists is slightly different, as there's no ptp_tx_lock there, the "problem" there is trying to enqueue a job on a workqueue that is going to be uninitialized, during this time window. And to be sure, I am still uncertain that this is possible. > >> If there's anything that makes this impossible/extremely unlikely, the >> patch looks good: >> >> Acked-by: Vinicius Costa Gomes >> >> >> Cheers, >> -- >> Vinicius > > > Corinna > Cheers, -- Vinicius From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E26FF12C559 for ; Thu, 25 Apr 2024 18:46:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.12 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714070789; cv=none; b=WFuOCnKJ6+/VHM4/0KbQAkEbhFsXKPdCd8zpcHWjH/tkMfjzEetWHbf5V5Kdiot+Vd8nf2aq61CQgXSXNQNEYg8cbsZVDw8iJ3BUjwGUQ26QcSBdP4ZupAH9AkFpS1evK4qpYB7cHCSdXn02zqIDM5kP5FAdyOqCeLw0M3vssAk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714070789; c=relaxed/simple; bh=ZsDqYu4ojuh9VuiJbaNfZoanFnnda5qja2yT5h3wsto=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=VoRg7UXgCrYAkKuoNIqq5cgSXwmUcpPDcDolJpRVJN6/356CfNFSQajVdKqeGLeRYK3UcZP3voP/SpyEub0SiEqwLr0U16DlwWAud3J62Zr8RSGA+alGsbeJ2VS+SCnm7aSxbqAgKBVnB6KEsz7hM+OGcLgn4BKYB4F9XVtRTAM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=EJeIoP+f; arc=none smtp.client-ip=192.198.163.12 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="EJeIoP+f" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1714070788; x=1745606788; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=ZsDqYu4ojuh9VuiJbaNfZoanFnnda5qja2yT5h3wsto=; b=EJeIoP+f9DsIGS1QQpRext0zb0YEXuTXYmYss6314OJ6RhOtfwwVNBfK wFK0x2CSenhivThGtZpnKvjnzOJ8iXidJUuNyY3fqg6D1WiYGzdjpqaFi H7GHQZNhaw2sWoUvNEwqcBvcIVxkGMD9XaMD6evHvkPhLk52pWKXGncjO DE6JeU72tCRBGAiuZMHhyD4f5mQ7jgqsN/S5hcS7ynF6VYyXs5NXHrAGe DNSLd7C843Wwifh8G/AegWuNkLB6R5ob1NT3ReKi9jg553Pmh/zlqsMn3 lXiu9lB+pTJRs/bm3DIhO6/IZdHnUmtd2aMtY93RPfw0DJMS6j06dfPT1 w==; X-CSE-ConnectionGUID: lhCNUF7LRCG/m3AhFkjGCA== X-CSE-MsgGUID: T1kQVsMKR22ynHVAnVl69w== X-IronPort-AV: E=McAfee;i="6600,9927,11055"; a="13569734" X-IronPort-AV: E=Sophos;i="6.07,230,1708416000"; d="scan'208";a="13569734" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Apr 2024 11:46:27 -0700 X-CSE-ConnectionGUID: pkPPmTyARQKBAQElP1o99A== X-CSE-MsgGUID: itgeq33yQP+DFUv66C/KQQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,230,1708416000"; d="scan'208";a="25666087" Received: from unknown (HELO vcostago-mobl3) ([10.124.220.196]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Apr 2024 11:46:27 -0700 From: Vinicius Costa Gomes To: Corinna Vinschen Cc: Andrew Lunn , netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org Subject: Re: [Intel-wired-lan] [PATCH] igc: fix a log entry using uninitialized netdev In-Reply-To: References: <20240423102455.901469-1-vinschen@redhat.com> <033cce07-fe8f-42e6-8c27-7afee87fe13c@lunn.ch> <8734raxq4z.fsf@intel.com> Date: Thu, 25 Apr 2024 11:46:26 -0700 Message-ID: <87r0etwa9p.fsf@intel.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain Corinna Vinschen writes: > On Apr 24 17:06, Vinicius Costa Gomes wrote: >> Andrew Lunn writes: >> >> > On Tue, Apr 23, 2024 at 12:24:54PM +0200, Corinna Vinschen wrote: >> >> During successful probe, igc logs this: >> >> >> >> [ 5.133667] igc 0000:01:00.0 (unnamed net_device) (uninitialized): PHC added >> >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> >> The reason is that igc_ptp_init() is called very early, even before >> >> register_netdev() has been called. So the netdev_info() call works >> >> on a partially uninitialized netdev. >> >> >> >> Fix this by calling igc_ptp_init() after register_netdev(), right >> >> after the media autosense check, just as in igb. Add a comment, >> >> just as in igb. >> > >> > The network stack can start sending and receiving packet before >> > register_netdev() returns. This is typical of NFS root for example. Is >> > there anything in igc_ptp_init() which could cause such packet >> > transfers to explode? >> > >> >> There might be a very narrow window (probably impossible?), what I can >> see is: >> >> 1. the netdevice is exposed to userspace; >> 2. userspace does the SIOCSHWTSTAMP ioctl() to enable TX timestamps; >> 3. userspace sends a packet that is going to be timestamped; >> >> if this happens before igc_ptp_init() is called, adapter->ptp_tx_lock is >> going to be uninitialized, and (3) is going to crash. > > The same would then be possible on igb as well, wouldn't it? > Given how many years igb is being used, perhaps "possible" is too strong :-) On igb what exists is slightly different, as there's no ptp_tx_lock there, the "problem" there is trying to enqueue a job on a workqueue that is going to be uninitialized, during this time window. And to be sure, I am still uncertain that this is possible. > >> If there's anything that makes this impossible/extremely unlikely, the >> patch looks good: >> >> Acked-by: Vinicius Costa Gomes >> >> >> Cheers, >> -- >> Vinicius > > > Corinna > Cheers, -- Vinicius