From: Simon Horman <horms@kernel.org>
To: Yuan Can <yuancan@huawei.com>
Cc: anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, cramerj@intel.com,
shannon.nelson@amd.com, mitch.a.williams@intel.com,
jgarzik@redhat.com, auke-jan.h.kok@intel.com,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
Alexander Duyck <alexander.duyck@gmail.com>
Subject: Re: [Intel-wired-lan] [PATCH] igb: Fix potential invalid memory access in igb_init_module()
Date: Tue, 22 Oct 2024 16:56:30 +0100 [thread overview]
Message-ID: <20241022155630.GY402847@kernel.org> (raw)
In-Reply-To: <20241022063807.37561-1-yuancan@huawei.com>
+ Alexander Duyck
On Tue, Oct 22, 2024 at 02:38:07PM +0800, Yuan Can wrote:
> The pci_register_driver() can fail and when this happened, the dca_notifier
> needs to be unregistered, otherwise the dca_notifier can be called when
> igb fails to install, resulting to invalid memory access.
>
> Fixes: fe4506b6a2f9 ("igb: add DCA support")
I don't think this problem was introduced by the commit cited above,
as it added the call to dca_unregister_notify() before
pci_register_driver(). But rather by the commit cited below which reversed
the order of these function calls.
bbd98fe48a43 ("igb: Fix DCA errors and do not use context index for 82576")
I'm unsure if it is necessary to repost the patch to address that.
But if you do, and assuming we are treating this as a bug fix,
please target it for the net (or iwl-net) tree like this:
Subject: [PATCH net v2] ...
> Signed-off-by: Yuan Can <yuancan@huawei.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index f1d088168723..18284a838e24 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -637,6 +637,10 @@ static int __init igb_init_module(void)
> dca_register_notify(&dca_notifier);
> #endif
> ret = pci_register_driver(&igb_driver);
> +#ifdef CONFIG_IGB_DCA
> + if (ret)
> + dca_unregister_notify(&dca_notifier);
> +#endif
> return ret;
> }
>
> --
> 2.17.1
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@kernel.org>
To: Yuan Can <yuancan@huawei.com>
Cc: anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, cramerj@intel.com,
shannon.nelson@amd.com, mitch.a.williams@intel.com,
jgarzik@redhat.com, auke-jan.h.kok@intel.com,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
Alexander Duyck <alexander.duyck@gmail.com>
Subject: Re: [PATCH] igb: Fix potential invalid memory access in igb_init_module()
Date: Tue, 22 Oct 2024 16:56:30 +0100 [thread overview]
Message-ID: <20241022155630.GY402847@kernel.org> (raw)
In-Reply-To: <20241022063807.37561-1-yuancan@huawei.com>
+ Alexander Duyck
On Tue, Oct 22, 2024 at 02:38:07PM +0800, Yuan Can wrote:
> The pci_register_driver() can fail and when this happened, the dca_notifier
> needs to be unregistered, otherwise the dca_notifier can be called when
> igb fails to install, resulting to invalid memory access.
>
> Fixes: fe4506b6a2f9 ("igb: add DCA support")
I don't think this problem was introduced by the commit cited above,
as it added the call to dca_unregister_notify() before
pci_register_driver(). But rather by the commit cited below which reversed
the order of these function calls.
bbd98fe48a43 ("igb: Fix DCA errors and do not use context index for 82576")
I'm unsure if it is necessary to repost the patch to address that.
But if you do, and assuming we are treating this as a bug fix,
please target it for the net (or iwl-net) tree like this:
Subject: [PATCH net v2] ...
> Signed-off-by: Yuan Can <yuancan@huawei.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index f1d088168723..18284a838e24 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -637,6 +637,10 @@ static int __init igb_init_module(void)
> dca_register_notify(&dca_notifier);
> #endif
> ret = pci_register_driver(&igb_driver);
> +#ifdef CONFIG_IGB_DCA
> + if (ret)
> + dca_unregister_notify(&dca_notifier);
> +#endif
> return ret;
> }
>
> --
> 2.17.1
>
>
next prev parent reply other threads:[~2024-10-22 15:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-22 6:38 [Intel-wired-lan] [PATCH] igb: Fix potential invalid memory access in igb_init_module() Yuan Can
2024-10-22 6:38 ` Yuan Can
2024-10-22 15:56 ` Simon Horman [this message]
2024-10-22 15:56 ` Simon Horman
2024-10-22 17:25 ` [Intel-wired-lan] " Alexander Duyck
2024-10-22 17:25 ` Alexander Duyck
2024-10-23 9:08 ` [Intel-wired-lan] " Yuan Can
2024-10-23 9:08 ` Yuan Can
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241022155630.GY402847@kernel.org \
--to=horms@kernel.org \
--cc=alexander.duyck@gmail.com \
--cc=andrew+netdev@lunn.ch \
--cc=anthony.l.nguyen@intel.com \
--cc=auke-jan.h.kok@intel.com \
--cc=cramerj@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jgarzik@redhat.com \
--cc=kuba@kernel.org \
--cc=mitch.a.williams@intel.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=przemyslaw.kitszel@intel.com \
--cc=shannon.nelson@amd.com \
--cc=yuancan@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.