linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iwlegacy: make il3945_mac_ops __ro_after_init
@ 2016-12-07  6:36 Johannes Berg
  2016-12-07 10:53 ` Stanislaw Gruszka
  2016-12-30 11:29 ` Kalle Valo
  0 siblings, 2 replies; 3+ messages in thread
From: Johannes Berg @ 2016-12-07  6:36 UTC (permalink / raw)
  To: linux-wireless; +Cc: Stanislaw Gruszka, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There's no need for this to be only __read_mostly, since
it's only written in a single way depending on the module
parameter, so that can be moved into the module's __init
function, and the ops can be __ro_after_init.

This is a little bit safer since it means the ops can't
be overwritten (accidentally or otherwise), which would
otherwise cause an arbitrary function or bad pointer to
be called.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/intel/iwlegacy/3945-mac.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/3945-mac.c b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
index 466912eb2d87..e8e65115feba 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
@@ -3469,7 +3469,7 @@ static struct attribute_group il3945_attribute_group = {
 	.attrs = il3945_sysfs_entries,
 };
 
-static struct ieee80211_ops il3945_mac_ops __read_mostly = {
+static struct ieee80211_ops il3945_mac_ops __ro_after_init = {
 	.tx = il3945_mac_tx,
 	.start = il3945_mac_start,
 	.stop = il3945_mac_stop,
@@ -3627,15 +3627,6 @@ il3945_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	il->cmd_queue = IL39_CMD_QUEUE_NUM;
 
-	/*
-	 * Disabling hardware scan means that mac80211 will perform scans
-	 * "the hard way", rather than using device's scan.
-	 */
-	if (il3945_mod_params.disable_hw_scan) {
-		D_INFO("Disabling hw_scan\n");
-		il3945_mac_ops.hw_scan = NULL;
-	}
-
 	D_INFO("*** LOAD DRIVER ***\n");
 	il->cfg = cfg;
 	il->ops = &il3945_ops;
@@ -3913,6 +3904,15 @@ il3945_init(void)
 	pr_info(DRV_DESCRIPTION ", " DRV_VERSION "\n");
 	pr_info(DRV_COPYRIGHT "\n");
 
+	/*
+	 * Disabling hardware scan means that mac80211 will perform scans
+	 * "the hard way", rather than using device's scan.
+	 */
+	if (il3945_mod_params.disable_hw_scan) {
+		pr_info("hw_scan is disabled\n");
+		il3945_mac_ops.hw_scan = NULL;
+	}
+
 	ret = il3945_rate_control_register();
 	if (ret) {
 		pr_err("Unable to register rate control algorithm: %d\n", ret);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] iwlegacy: make il3945_mac_ops __ro_after_init
  2016-12-07  6:36 [PATCH] iwlegacy: make il3945_mac_ops __ro_after_init Johannes Berg
@ 2016-12-07 10:53 ` Stanislaw Gruszka
  2016-12-30 11:29 ` Kalle Valo
  1 sibling, 0 replies; 3+ messages in thread
From: Stanislaw Gruszka @ 2016-12-07 10:53 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Wed, Dec 07, 2016 at 07:36:46AM +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> There's no need for this to be only __read_mostly, since
> it's only written in a single way depending on the module
> parameter, so that can be moved into the module's __init
> function, and the ops can be __ro_after_init.
> 
> This is a little bit safer since it means the ops can't
> be overwritten (accidentally or otherwise), which would
> otherwise cause an arbitrary function or bad pointer to
> be called.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: iwlegacy: make il3945_mac_ops __ro_after_init
  2016-12-07  6:36 [PATCH] iwlegacy: make il3945_mac_ops __ro_after_init Johannes Berg
  2016-12-07 10:53 ` Stanislaw Gruszka
@ 2016-12-30 11:29 ` Kalle Valo
  1 sibling, 0 replies; 3+ messages in thread
From: Kalle Valo @ 2016-12-30 11:29 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Stanislaw Gruszka, Johannes Berg

Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> There's no need for this to be only __read_mostly, since
> it's only written in a single way depending on the module
> parameter, so that can be moved into the module's __init
> function, and the ops can be __ro_after_init.
> 
> This is a little bit safer since it means the ops can't
> be overwritten (accidentally or otherwise), which would
> otherwise cause an arbitrary function or bad pointer to
> be called.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>

Patch applied to wireless-drivers-next.git, thanks.

ae3cf4764506 iwlegacy: make il3945_mac_ops __ro_after_init

-- 
https://patchwork.kernel.org/patch/9463999/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-12-30 11:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-07  6:36 [PATCH] iwlegacy: make il3945_mac_ops __ro_after_init Johannes Berg
2016-12-07 10:53 ` Stanislaw Gruszka
2016-12-30 11:29 ` Kalle Valo

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).