* RE: [PATCH 5.1] rtw88: fix subscript above array bounds compiler warning
From: Tony Chuang @ 2019-05-06 6:32 UTC (permalink / raw)
To: Stanislaw Gruszka, linux-wireless@vger.kernel.org
In-Reply-To: <20190506062358.8288-1-sgruszka@redhat.com>
> Subject: [PATCH 5.1] rtw88: fix subscript above array bounds compiler warning
>
> My compiler complains about:
>
> drivers/net/wireless/realtek/rtw88/phy.c: In function
> ‘rtw_phy_rf_power_2_rssi’:
> drivers/net/wireless/realtek/rtw88/phy.c:430:26: warning: array subscript is
> above array bounds [-Warray-bounds]
> linear = db_invert_table[i][j];
>
> According to comment power_db should be in range 1 ~ 96 .
> To fix add check for boundaries before access the array.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> RFC -> v1
> - add check before accessing the array insted of
> rtw_phy_power_2_db() change.
>
> drivers/net/wireless/realtek/rtw88/phy.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/phy.c
> b/drivers/net/wireless/realtek/rtw88/phy.c
> index 4381b360b5b5..9ca52a4d025a 100644
> --- a/drivers/net/wireless/realtek/rtw88/phy.c
> +++ b/drivers/net/wireless/realtek/rtw88/phy.c
> @@ -423,6 +423,11 @@ static u64 rtw_phy_db_2_linear(u8 power_db)
> u8 i, j;
> u64 linear;
>
> + if (power_db > 96)
> + power_db = 96;
> + else if (power_db < 1)
> + power_db = 1;
I think it's "return 1" here.
> +
> /* 1dB ~ 96dB */
> i = (power_db - 1) >> 3;
> j = (power_db - 1) - (i << 3);
> --
Yan-Hsuan
^ permalink raw reply
* [PATCH 5.1] rtw88: fix subscript above array bounds compiler warning
From: Stanislaw Gruszka @ 2019-05-06 6:23 UTC (permalink / raw)
To: linux-wireless; +Cc: Yan-Hsuan Chuang
My compiler complains about:
drivers/net/wireless/realtek/rtw88/phy.c: In function ‘rtw_phy_rf_power_2_rssi’:
drivers/net/wireless/realtek/rtw88/phy.c:430:26: warning: array subscript is above array bounds [-Warray-bounds]
linear = db_invert_table[i][j];
According to comment power_db should be in range 1 ~ 96 .
To fix add check for boundaries before access the array.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
RFC -> v1
- add check before accessing the array insted of
rtw_phy_power_2_db() change.
drivers/net/wireless/realtek/rtw88/phy.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
index 4381b360b5b5..9ca52a4d025a 100644
--- a/drivers/net/wireless/realtek/rtw88/phy.c
+++ b/drivers/net/wireless/realtek/rtw88/phy.c
@@ -423,6 +423,11 @@ static u64 rtw_phy_db_2_linear(u8 power_db)
u8 i, j;
u64 linear;
+ if (power_db > 96)
+ power_db = 96;
+ else if (power_db < 1)
+ power_db = 1;
+
/* 1dB ~ 96dB */
i = (power_db - 1) >> 3;
j = (power_db - 1) - (i << 3);
--
2.20.1
^ permalink raw reply related
* RE: [PATCH] rtw88: avoid circular locking between local->iflist_mtx and rtwdev->mutex
From: Tony Chuang @ 2019-05-06 5:22 UTC (permalink / raw)
To: Stanislaw Gruszka, linux-wireless@vger.kernel.org
In-Reply-To: <1556886547-23632-1-git-send-email-sgruszka@redhat.com>
> Subject: [PATCH] rtw88: avoid circular locking between local->iflist_mtx and
> rtwdev->mutex
>
> Remove circular lock dependency by using atomic version of interfaces
> iterate in watch_dog_work(), hence avoid taking local->iflist_mtx
> (rtw_vif_watch_dog_iter() only update some data, it can be called from
> atomic context). Fixes below LOCKDEP warning:
>
> [ 1157.219415]
> ======================================================
> [ 1157.225772] [ INFO: possible circular locking dependency detected ]
> [ 1157.232150] 3.10.0-1043.el7.sgruszka1.x86_64.debug #1 Not tainted
> [ 1157.238346] -------------------------------------------------------
> [ 1157.244635] kworker/u4:2/14490 is trying to acquire lock:
> [ 1157.250194] (&rtwdev->mutex){+.+.+.}, at: [<ffffffffc098322b>]
> rtw_ops_config+0x2b/0x90 [rtw88]
> [ 1157.259151]
> but task is already holding lock:
> [ 1157.265085] (&local->iflist_mtx){+.+...}, at: [<ffffffffc0b8ab7a>]
> ieee80211_mgd_probe_ap.part.28+0xca/0x160 [mac80211]
> [ 1157.276169]
> which lock already depends on the new lock.
>
> [ 1157.284488]
> the existing dependency chain (in reverse order) is:
> [ 1157.292101]
> -> #2 (&local->iflist_mtx){+.+...}:
> [ 1157.296919] [<ffffffffbc741a29>] lock_acquire+0x99/0x1e0
> [ 1157.302955] [<ffffffffbce72793>] mutex_lock_nested+0x93/0x410
> [ 1157.309416] [<ffffffffc0b6038f>]
> ieee80211_iterate_interfaces+0x2f/0x60 [mac80211]
> [ 1157.317730] [<ffffffffc09811ab>]
> rtw_watch_dog_work+0xcb/0x130 [rtw88]
> [ 1157.325003] [<ffffffffbc6d77bc>] process_one_work+0x22c/0x720
> [ 1157.331481] [<ffffffffbc6d7dd6>] worker_thread+0x126/0x3b0
> [ 1157.337589] [<ffffffffbc6e107f>] kthread+0xef/0x100
> [ 1157.343260] [<ffffffffbce848b7>]
> ret_from_fork_nospec_end+0x0/0x39
> [ 1157.350091]
> -> #1 ((&(&rtwdev->watch_dog_work)->work)){+.+...}:
> [ 1157.356314] [<ffffffffbc741a29>] lock_acquire+0x99/0x1e0
> [ 1157.362427] [<ffffffffbc6d570b>] flush_work+0x5b/0x310
> [ 1157.368287] [<ffffffffbc6d740e>]
> __cancel_work_timer+0xae/0x170
> [ 1157.374940] [<ffffffffbc6d7583>]
> cancel_delayed_work_sync+0x13/0x20
> [ 1157.381930] [<ffffffffc0982b49>] rtw_core_stop+0x29/0x50
> [rtw88]
> [ 1157.388679] [<ffffffffc098bee6>] rtw_enter_ips+0x16/0x20
> [rtw88]
> [ 1157.395428] [<ffffffffc0983242>] rtw_ops_config+0x42/0x90
> [rtw88]
> [ 1157.402173] [<ffffffffc0b13343>]
> ieee80211_hw_config+0xc3/0x680 [mac80211]
> [ 1157.409854] [<ffffffffc0b3925b>]
> ieee80211_do_open+0x69b/0x9c0 [mac80211]
> [ 1157.417418] [<ffffffffc0b395e9>] ieee80211_open+0x69/0x70
> [mac80211]
> [ 1157.424496] [<ffffffffbcd03442>] __dev_open+0xe2/0x160
> [ 1157.430356] [<ffffffffbcd03773>]
> __dev_change_flags+0xa3/0x180
> [ 1157.436922] [<ffffffffbcd03879>] dev_change_flags+0x29/0x60
> [ 1157.443224] [<ffffffffbcda14c4>] devinet_ioctl+0x794/0x890
> [ 1157.449331] [<ffffffffbcda27b5>] inet_ioctl+0x75/0xa0
> [ 1157.455087] [<ffffffffbccd54eb>] sock_do_ioctl+0x2b/0x60
> [ 1157.461178] [<ffffffffbccd5753>] sock_ioctl+0x233/0x310
> [ 1157.467109] [<ffffffffbc8bd820>] do_vfs_ioctl+0x410/0x6c0
> [ 1157.473233] [<ffffffffbc8bdb71>] SyS_ioctl+0xa1/0xc0
> [ 1157.478914] [<ffffffffbce84a5e>] system_call_fastpath+0x25/0x2a
> [ 1157.485569]
> -> #0 (&rtwdev->mutex){+.+.+.}:
> [ 1157.490022] [<ffffffffbc7409d1>] __lock_acquire+0xec1/0x1630
> [ 1157.496305] [<ffffffffbc741a29>] lock_acquire+0x99/0x1e0
> [ 1157.502413] [<ffffffffbce72793>] mutex_lock_nested+0x93/0x410
> [ 1157.508890] [<ffffffffc098322b>] rtw_ops_config+0x2b/0x90
> [rtw88]
> [ 1157.515724] [<ffffffffc0b13343>]
> ieee80211_hw_config+0xc3/0x680 [mac80211]
> [ 1157.523370] [<ffffffffc0b8a4ca>]
> ieee80211_recalc_ps.part.27+0x9a/0x180 [mac80211]
> [ 1157.531685] [<ffffffffc0b8abc5>]
> ieee80211_mgd_probe_ap.part.28+0x115/0x160 [mac80211]
> [ 1157.540353] [<ffffffffc0b8b40d>]
> ieee80211_beacon_connection_loss_work+0x4d/0x80 [mac80211]
> [ 1157.549513] [<ffffffffbc6d77bc>] process_one_work+0x22c/0x720
> [ 1157.555886] [<ffffffffbc6d7dd6>] worker_thread+0x126/0x3b0
> [ 1157.562170] [<ffffffffbc6e107f>] kthread+0xef/0x100
> [ 1157.567765] [<ffffffffbce848b7>]
> ret_from_fork_nospec_end+0x0/0x39
> [ 1157.574579]
> other info that might help us debug this:
>
> [ 1157.582788] Chain exists of:
> &rtwdev->mutex --> (&(&rtwdev->watch_dog_work)->work) -->
> &local->iflist_mtx
>
> [ 1157.593024] Possible unsafe locking scenario:
>
> [ 1157.599046] CPU0 CPU1
> [ 1157.603653] ---- ----
> [ 1157.608258] lock(&local->iflist_mtx);
> [ 1157.612180]
> lock((&(&rtwdev->watch_dog_work)->work));
> [ 1157.620074] lock(&local->iflist_mtx);
> [ 1157.626555] lock(&rtwdev->mutex);
> [ 1157.630124]
> *** DEADLOCK ***
>
> [ 1157.636148] 4 locks held by kworker/u4:2/14490:
> [ 1157.640755] #0: (%s#6){.+.+.+}, at: [<ffffffffbc6d774a>]
> process_one_work+0x1ba/0x720
> [ 1157.648965] #1: ((&ifmgd->beacon_connection_loss_work)){+.+.+.}, at:
> [<ffffffffbc6d774a>] process_one_work+0x1ba/0x720
> [ 1157.659950] #2: (&wdev->mtx){+.+.+.}, at: [<ffffffffc0b8aad5>]
> ieee80211_mgd_probe_ap.part.28+0x25/0x160 [mac80211]
> [ 1157.670901] #3: (&local->iflist_mtx){+.+...}, at: [<ffffffffc0b8ab7a>]
> ieee80211_mgd_probe_ap.part.28+0xca/0x160 [mac80211]
> [ 1157.682466]
>
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> drivers/net/wireless/realtek/rtw88/main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/main.c
> b/drivers/net/wireless/realtek/rtw88/main.c
> index 9893e5e297e3..f63c34a2e356 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.c
> +++ b/drivers/net/wireless/realtek/rtw88/main.c
> @@ -162,7 +162,8 @@ static void rtw_watch_dog_work(struct work_struct
> *work)
> rtwdev->stats.tx_cnt = 0;
> rtwdev->stats.rx_cnt = 0;
>
> - rtw_iterate_vifs(rtwdev, rtw_vif_watch_dog_iter, &data);
> + /* use atomic version to avoid taking local->iflist_mtx mutex */
> + rtw_iterate_vifs_atomic(rtwdev, rtw_vif_watch_dog_iter, &data);
>
> /* fw supports only one station associated to enter lps, if there are
> * more than two stations associated to the AP, then we can not enter
> --
Looks good to me.
Thanks.
Acked-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
Yan-Hsuan
^ permalink raw reply
* Re: [PATCH][next] brcmfmac: remove redundant u32 comparison with less than zero
From: Rafał Miłecki @ 2019-05-06 4:11 UTC (permalink / raw)
To: Colin King, Wright Feng
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Kalle Valo,
David S . Miller, linux-wireless@vger.kernel.org,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER <brcm80211-dev-list.pdl@broadcom.com>,,
Network Development, kernel-janitors, Linux Kernel Mailing List
In-Reply-To: <20190505211623.3153-1-colin.king@canonical.com>
On Sun, 5 May 2019 at 23:33, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The check for the u32 variable idx being less than zero is
> always false and is redundant. Remove it.
>
> Addresses-Coverity: ("Unsigned compared against 0")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> index 9d1f9ff25bfa..e874dddc7b7f 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> @@ -375,7 +375,7 @@ brcmf_msgbuf_get_pktid(struct device *dev, struct brcmf_msgbuf_pktids *pktids,
> struct brcmf_msgbuf_pktid *pktid;
> struct sk_buff *skb;
>
> - if (idx < 0 || idx >= pktids->array_size) {
> + if (idx >= pktids->array_size) {
> brcmf_err("Invalid packet id %d (max %d)\n", idx,
> pktids->array_size);
> return NULL;
It was added in the commit 2d91c8ad068a ("brcmfmac: set txflow request
id from 1 to pktids array size") and was probably meant to handle a
following brcmf_msgbuf_process_txstatus() case:
idx = le32_to_cpu(tx_status->msg.request_id) - 1;
So this patch is wrong/incomplete.
You should change that to a signed value OR add an extra check in
brcmf_msgbuf_process_txstatus() to make sure it doesn't pass -1 as u32
argument.
--
Rafał
^ permalink raw reply
* RE: [RFC] rtw88: fix subscript above array bounds compiler warning
From: Tony Chuang @ 2019-05-06 3:42 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <20190503122426.GA4423@redhat.com>
> Subject: Re: [RFC] rtw88: fix subscript above array bounds compiler warning
>
> On Fri, May 03, 2019 at 12:01:05PM +0000, Tony Chuang wrote:
> > > Subject: [RFC] rtw88: fix subscript above array bounds compiler warning
> > >
> > > My compiler complain about:
> > >
> > > drivers/net/wireless/realtek/rtw88/phy.c: In function
> > > ‘rtw_phy_rf_power_2_rssi’:
> > > drivers/net/wireless/realtek/rtw88/phy.c:430:26: warning: array subscript is
> > > above array bounds [-Warray-bounds]
> > > linear = db_invert_table[i][j];
> > >
> > > According to comment power_db should be in range 1 ~ 96 .
> > > Correct rtw_phy_power_2_db() to make max power 96 db
> > > (still min is 0). This make the warning gone.
> > >
> > > However power >= 20 check still looks somewhat suspicious to me.
> > >
> > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > > ---
> > > drivers/net/wireless/realtek/rtw88/phy.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/realtek/rtw88/phy.c
> > > b/drivers/net/wireless/realtek/rtw88/phy.c
> > > index 35a35dbca85f..a716a44d78b0 100644
> > > --- a/drivers/net/wireless/realtek/rtw88/phy.c
> > > +++ b/drivers/net/wireless/realtek/rtw88/phy.c
> > > @@ -410,12 +410,12 @@ void rtw_phy_dynamic_mechanism(struct
> rtw_dev
> > > *rtwdev)
> > >
> > > static u8 rtw_phy_power_2_db(s8 power)
> > > {
> > > - if (power <= -100 || power >= 20)
> > > + if (power <= -96 || power >= 20)
> > > return 0;
> > > else if (power >= 0)
> > > - return 100;
> > > + return 96;
> > > else
> > > - return 100 + power;
> > > + return 96 + power;
> > > }
> > >
> > > static u64 rtw_phy_db_2_linear(u8 power_db)
> > > --
> >
> > I think I should check with the radio team, that if the power from the
> > rx descriptor generated by hardware can possibly get >= 20
> >
> > And also check what the actual logic they expected to deal with the power.
> > Thanks for reporting it.
>
> Yeah, this could be just teoretical issue as we can not get power
> values >= 0 from HW. However I think compiler correctly complains, as
> for power_db=100 we get i = ((100 - 1) >> 3) = 12 , what exceed by one
> max first index of db_invert_table[][], which should be in range
> 0 - 11.
>
I checked it. The power sum could actually be like 20 or something.
And the recommended modification is to restrict the value used for array
subscript between 1~96 before access the array. Such as:
@@ -578,6 +578,11 @@ static u64 rtw_phy_db_2_linear(u8 power_db)
u8 i, j;
u64 linear;
+ if (power_db > 96)
+ power_db = 96;
+ else if (power_db < 1)
+ return 1;
+
/* 1dB ~ 96dB */
i = (power_db - 1) >> 3;
j = (power_db - 1) - (i << 3);
Yan-Hsuan
^ permalink raw reply
* Re: [PATCH v8 15/15] x86/split_lock: Add a sysfs interface to enable/disable split lock detection during run time
From: Fenghua Yu @ 2019-05-06 0:21 UTC (permalink / raw)
To: Ingo Molnar
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H Peter Anvin,
Paolo Bonzini, Dave Hansen, Ashok Raj, Peter Zijlstra,
Ravi V Shankar, Xiaoyao Li, Christopherson Sean J, Kalle Valo,
Michael Chan, linux-kernel, x86, kvm, netdev, linux-wireless
In-Reply-To: <20190425063115.GD40105@gmail.com>
On Thu, Apr 25, 2019 at 08:31:15AM +0200, Ingo Molnar wrote:
>
> * Fenghua Yu <fenghua.yu@intel.com> wrote:
>
> > + disabled if split lock operation in kernel code happens on
> > + the CPU. The interface doesn't show or control split lock
> > + detection on individual CPU.
>
> I.e. implementation and possible actual state are out of sync. Why?
>
> Also, if it's a global flag, why waste memory on putting a sysfs knob
> into every CPU's sysfs file?
>
> Finally, why is a debugging facility in sysfs, why not a debugfs knob?
> Using a sysctl would solve the percpu vs. global confusion as well ...
Can I put the interface in /sys/kernel/debug/x86/split_lock_detect?
>
> > --- a/arch/x86/kernel/cpu/intel.c
> > +++ b/arch/x86/kernel/cpu/intel.c
> > @@ -35,6 +35,7 @@
> > DEFINE_PER_CPU(u64, msr_test_ctl_cache);
> > EXPORT_PER_CPU_SYMBOL_GPL(msr_test_ctl_cache);
> >
> > +static DEFINE_MUTEX(split_lock_detect_mutex);
> > static bool split_lock_detect_enable;
>
> 'enable' is a verb in plain form - which we use for function names.
>
> For variable names that denotes current state we typically use past
> tense, i.e. 'enabled'.
>
> (The only case where we'd us the split_lock_detect_enable name for a flag
> if it's a flag to trigger some sort of enabling action - which this
> isn't.)
>
> Please review the whole series for various naming mishaps.
OK.
>
> > + mutex_lock(&split_lock_detect_mutex);
> > +
> > + split_lock_detect_enable = val;
> > +
> > + /* Update the split lock detection setting in MSR on all online CPUs. */
> > + on_each_cpu(split_lock_update_msr, NULL, 1);
> > +
> > + if (split_lock_detect_enable)
> > + pr_info("enabled\n");
> > + else
> > + pr_info("disabled\n");
> > +
> > + mutex_unlock(&split_lock_detect_mutex);
>
> Instead of a mutex, please just use the global atomic debug flag which
> controls the warning printout. By using that flag both for the WARN()ing
> and for controlling MSR state all the races are solved and the code is
> simplified.
So is it OK to define split_lock_debug and use it in #AC handler and in
here?
static atomic_t split_lock_debug;
in #AC handler:
+ if (atomic_cmpxchg(&split_lock_debug, 0, 1) == 0) {
+ /* Only warn split lock once */
+ WARN_ONCE(1, "split lock operation detected\n");
+ atomic_set(&split_lock_debug, 0);
+ }
And in split_lock_detect_store(), replace the mutex with split_lock_debug
like this:
- mutex_lock(&split_lock_detect_mutex);
+ while (atomic_cmpxchg(&split_lock_debug, 1, 0))
+ cpu_relax();
....
- mutex_unlock(&split_lock_detect_mutex);
+ atomic_set(&split_lock_debug, 0);
Is this right code for sync in both #AC handler and in
split_lock_detect_store()?
Thanks.
-Fenghua
^ permalink raw reply
* Re: [PATCH v8 05/15] x86/msr-index: Define MSR_IA32_CORE_CAPABILITY and split lock detection bit
From: Fenghua Yu @ 2019-05-06 0:12 UTC (permalink / raw)
To: Ingo Molnar
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H Peter Anvin,
Paolo Bonzini, Dave Hansen, Ashok Raj, Peter Zijlstra,
Ravi V Shankar, Xiaoyao Li, Christopherson Sean J, Kalle Valo,
Michael Chan, linux-kernel, x86, kvm, netdev, linux-wireless
In-Reply-To: <20190426060010.GB122831@gmail.com>
On Fri, Apr 26, 2019 at 08:00:10AM +0200, Ingo Molnar wrote:
>
> * Fenghua Yu <fenghua.yu@intel.com> wrote:
>
> > On Thu, Apr 25, 2019 at 10:08:30PM +0200, Ingo Molnar wrote:
> > >
> > > * Fenghua Yu <fenghua.yu@intel.com> wrote:
> > >
> > > > On Thu, Apr 25, 2019 at 09:47:14PM +0200, Ingo Molnar wrote:
> > > > >
> > > > > * Fenghua Yu <fenghua.yu@intel.com> wrote:
> > > > >
> > > > > > On Thu, Apr 25, 2019 at 07:45:11AM +0200, Ingo Molnar wrote:
> > > > > > >
> > > > > > > * Fenghua Yu <fenghua.yu@intel.com> wrote:
> > > > > > >
> > > > > > > > A new MSR_IA32_CORE_CAPABILITY (0xcf) is defined. Each bit in the MSR
> > > > > > > > enumerates a model specific feature. Currently bit 5 enumerates split
> > > > > > > > lock detection. When bit 5 is 1, split lock detection is supported.
> > > > > > > > When the bit is 0, split lock detection is not supported.
> > > > > > > >
> > > > > > > > Please check the latest Intel 64 and IA-32 Architectures Software
> > > > > > > > Developer's Manual for more detailed information on the MSR and the
> > > > > > > > split lock detection bit.
> > > > > > > >
> > > > > > > > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> > > > > > > > ---
> > > > > > > > arch/x86/include/asm/msr-index.h | 3 +++
> > > > > > > > 1 file changed, 3 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> > > > > > > > index ca5bc0eacb95..f65ef6f783d2 100644
> > > > > > > > --- a/arch/x86/include/asm/msr-index.h
> > > > > > > > +++ b/arch/x86/include/asm/msr-index.h
> > > > > > > > @@ -59,6 +59,9 @@
> > > > > > > > #define MSR_PLATFORM_INFO_CPUID_FAULT_BIT 31
> > > > > > > > #define MSR_PLATFORM_INFO_CPUID_FAULT BIT_ULL(MSR_PLATFORM_INFO_CPUID_FAULT_BIT)
> > > > > > > >
> > > > > > > > +#define MSR_IA32_CORE_CAPABILITY 0x000000cf
> > > > > > > > +#define CORE_CAP_SPLIT_LOCK_DETECT BIT(5) /* Detect split lock */
> > > > > > >
> > > > > > > Please don't put comments into definitions.
> > > > > >
> > > > > > I'll remove the comment and change definitions of the MSR and the split lock
> > > > > > detection bit as following:
> > > > > >
> > > > > > +#define MSR_IA32_CORE_CAPABILITY 0x000000cf
> > > > > > +#define MSR_IA32_CORE_CAPABILITY_SPLIT_LOCK_DETECT_BIT 5
> > > > > > +#define MSR_IA32_CORE_CAPABILITY_SPLIT_LOCK_DETECT BIT(MSR_IA32_CORE_CAPABILITY_SPLIT_LOCK_DETECT_BIT)
> > > > > >
> > > > > > Are these right changes?
> > > > >
> > > > > I suspect it could be shortened to CORE_CAP as you (partly) did it
> > > > > originally.
> > > >
> > > > IA32_CORE_CAPABILITY is the MSR's exact name in the latest SDM (in Table 2-14):
> > > > https://software.intel.com/en-us/download/intel-64-and-ia-32-architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4
> > > >
> > > > So can I define the MSR and the bits as follows?
> > > >
> > > > +#define MSR_IA32_CORE_CAP 0x000000cf
> > > > +#define MSR_IA32_CORE_CAP_SPLIT_LOCK_DETECT_BIT 5
> > > > +#define MSR_IA32_CORE_CAP_SPLIT_LOCK_DETECT BIT(MSR_IA32_CORE_CAP_SPLIT_LOCK_DETECT_BIT)
> > >
> > > Yeah, I suppose that looks OK.
> >
> > Should I also change the feature definition 'X86_FEATURE_CORE_CAPABILITY' to
> > 'X86_FEATURE_CORE_CAP' in cpufeatures.h in patch #0006 to match the
> > MSR definition here? Or should I still keep the current feature definition?
> >
> > Thanks.
>
> Hm, no, for CPU features it's good to follow the vendor convention.
>
> So I guess the long-form CPU_CAPABILITY for all of these is the best
> after all.
Since MSR_IA32_CORE_CAP_SPLIT_LOCK_DETECT_BIT is not used anywhere else
except in this patch, is it OK not to define this macro?
So this patch will only has two shorter lines:
+#define MSR_IA32_CORE_CAP 0x000000cf
+#define MSR_IA32_CORE_CAP_SPLIT_LOCK_DETECT BIT(5)
Is this OK for this patch to only define these two macros?
Thanks.
-Fenghua
^ permalink raw reply
* [PATCH][next] mt76: fix less than zero check on a u8 variable
From: Colin King @ 2019-05-05 21:31 UTC (permalink / raw)
To: Felix Fietkau, Roy Luo, Kalle Valo, David S . Miller,
Matthias Brugger, linux-wireless, netdev, linux-arm-kernel,
linux-mediatek
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The signed return from the call to get_omac_idx is being assigned to the
u8 variable mac_idx and then checked for a negative error condition
which is always going to be false. Fix this by assigning the return to
the int variable ret and checking this instead.
Addresses-Coverity: ("Unsigned compared against 0")
Fixes: 04b8e65922f6 ("mt76: add mac80211 driver for MT7615 PCIe-based chipsets")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/mediatek/mt76/mt7615/main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/main.c b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
index 80e6b211f60b..460d90d5ed6d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/main.c
@@ -77,11 +77,12 @@ static int mt7615_add_interface(struct ieee80211_hw *hw,
goto out;
}
- mvif->omac_idx = get_omac_idx(vif->type, dev->omac_mask);
- if (mvif->omac_idx < 0) {
+ ret = get_omac_idx(vif->type, dev->omac_mask);
+ if (ret < 0) {
ret = -ENOSPC;
goto out;
}
+ mvif->omac_idx = ret;
/* TODO: DBDC support. Use band 0 and wmm 0 for now */
mvif->band_idx = 0;
--
2.20.1
^ permalink raw reply related
* [PATCH][next] brcmfmac: remove redundant u32 comparison with less than zero
From: Colin King @ 2019-05-05 21:16 UTC (permalink / raw)
To: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Kalle Valo, David S . Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The check for the u32 variable idx being less than zero is
always false and is redundant. Remove it.
Addresses-Coverity: ("Unsigned compared against 0")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
index 9d1f9ff25bfa..e874dddc7b7f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
@@ -375,7 +375,7 @@ brcmf_msgbuf_get_pktid(struct device *dev, struct brcmf_msgbuf_pktids *pktids,
struct brcmf_msgbuf_pktid *pktid;
struct sk_buff *skb;
- if (idx < 0 || idx >= pktids->array_size) {
+ if (idx >= pktids->array_size) {
brcmf_err("Invalid packet id %d (max %d)\n", idx,
pktids->array_size);
return NULL;
--
2.20.1
^ permalink raw reply related
* Re: pull-request: wireless-drivers-next 2019-05-03
From: David Miller @ 2019-05-05 17:21 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87bm0jk9tv.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Fri, 03 May 2019 15:21:16 +0300
> here's a pull request to net-next for 5.2, more information below. Also
> there's a conflict in iwlwifi again and this time I added the conflict
> resolution to the signed tag based on our discussion earlier this week.
> Please let me know what you think or there are any problems.
Pulled, looks great, thanks Kalle.
^ permalink raw reply
* Re: [PATCH 12/17] mt7615: mcu: do not use function pointers whenever possible
From: Lorenzo Bianconi @ 2019-05-05 15:21 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, Ryder Lee, royluo
In-Reply-To: <de228dd80ba394aeb79ed9987e34cb37b495509a.1556981521.git.lorenzo@kernel.org>
>
> Remove function pointers in mt7615_mcu_set_bss_info and run function
> directly. Moreover remove __mt7615_mcu_set_bss_info since it is run just
> by mt7615_mcu_set_bss_info and remove duplicated istructions
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> .../net/wireless/mediatek/mt76/mt7615/mcu.c | 220 +++++++++---------
> 1 file changed, 105 insertions(+), 115 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> index 0e82fcb34e07..0632b506dd57 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> @@ -676,154 +676,107 @@ int mt7615_mcu_set_dev_info(struct mt7615_dev *dev,
> return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_DEV_INFO_UPDATE);
> }
>
[...]
> @@ -832,15 +785,52 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
> WARN_ON(1);
> break;
> }
> - memcpy(bss_info.bssid, vif->bss_conf.bssid, ETH_ALEN);
>
> - if (en) {
> - bss_info.feature |= BIT(BSS_INFO_OMAC);
> - if (mvif->omac_idx > EXT_BSSID_START)
> - bss_info.feature |= BIT(BSS_INFO_EXT_BSS);
> + buf = kzalloc(len, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + hdr = (struct req_hdr *)buf;
> + hdr->bss_idx = mvif->idx;
> + hdr->tlv_num = cpu_to_le16(ntlv);
> + hdr->is_tlv_append = 1;
> +
> + data = buf + sizeof(*hdr);
> + for (i = 0; i < BSS_INFO_MAX_NUM; i++) {
> + int tag = max_t(int, ffs(features & BIT(i)) - 1, 0);
> +
Ops, I found an issue here, it should be:
int tag = ffs(features & BIT(i)) - 1;
@Felix: do I need to resubmit the whole series or just a fix for this patch?
Regards,
Lorenzo
> + switch (tag) {
> + case BSS_INFO_OMAC:
> + mt7615_mcu_bss_info_omac_header(mvif, data,
> + conn_type);
> + data += sizeof(struct bss_info_omac);
> + break;
> + case BSS_INFO_BASIC:
> + mt7615_mcu_bss_info_basic_header(vif, data, net_type,
> + tx_wlan_idx, en);
> + data += sizeof(struct bss_info_basic);
> + break;
> + case BSS_INFO_EXT_BSS:
> + mt7615_mcu_bss_info_ext_header(mvif, data);
> + data += sizeof(struct bss_info_ext_bss);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + skb = mt7615_mcu_msg_alloc(buf, len);
> + if (!skb) {
> + ret = -ENOMEM;
> + goto out;
> }
>
> - return __mt7615_mcu_set_bss_info(dev, &bss_info);
> + ret = mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BSS_INFO_UPDATE);
> +
> +out:
> + kfree(buf);
> +
> + return ret;
> }
>
> static int
> --
> 2.20.1
>
^ permalink raw reply
* [PATCH] mt76: mt7615: rearrange cleanup operations in mt7615_unregister_device
From: Lorenzo Bianconi @ 2019-05-05 15:17 UTC (permalink / raw)
To: nbd; +Cc: lorenzo.bianconi, linux-wireless, ryder.lee, royluo
In-Reply-To: <cover.1557059004.git.lorenzo@kernel.org>
Cleanup tx/rx napi before releasing pending idrs.
Moreover unmap txwi_cache running mt76_free_device routine
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/net/wireless/mediatek/mt76/mt7615/init.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
index 3ab3ff553ef2..59f604f3161f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
@@ -212,6 +212,10 @@ void mt7615_unregister_device(struct mt7615_dev *dev)
struct mt76_txwi_cache *txwi;
int id;
+ mt76_unregister_device(&dev->mt76);
+ mt7615_dma_cleanup(dev);
+ mt7615_mcu_exit(dev);
+
spin_lock_bh(&dev->token_lock);
idr_for_each_entry(&dev->token, txwi, id) {
mt7615_txp_skb_unmap(&dev->mt76, txwi);
@@ -221,9 +225,6 @@ void mt7615_unregister_device(struct mt7615_dev *dev)
}
spin_unlock_bh(&dev->token_lock);
idr_destroy(&dev->token);
- mt76_unregister_device(&dev->mt76);
- mt7615_mcu_exit(dev);
- mt7615_dma_cleanup(dev);
- ieee80211_free_hw(mt76_hw(dev));
+ mt76_free_device(&dev->mt76);
}
--
2.20.1
^ permalink raw reply related
* [PATCH RFC] brcmfmac: sanitize DMI strings v2
From: Victor Bravo @ 2019-05-05 15:03 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Kalle Valo, David S. Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-kernel,
Hans de Goede
In-Reply-To: <16a87149068.2764.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>
Sanitize DMI strings in brcmfmac driver to make resulting filenames
contain only safe characters. This version replaces all non-printable
characters incl. delete (0-31, 127-255), spaces and slashes with
underscores.
This change breaks backward compatibility, but adds control over strings
passed to firmware loader and compatibility with CONFIG_EXTRA_FIRMWARE
which doesn't support spaces in filenames.
Changes from v1: don't revert fresh commit by someone else
Signed-off-by: Victor Bravo <1905@spmblk.com>
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
index 7535cb0d4ac0..84571e09b465 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
@@ -23,6 +23,14 @@
/* The DMI data never changes so we can use a static buf for this */
static char dmi_board_type[128];
+/* Array of 128 bits representing 7-bit characters allowed in DMI strings. */
+static unsigned char brcmf_dmi_allowed_chars[] = {
+ 0x00, 0x00, 0x00, 0x00, 0xfe, 0x7f, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f
+};
+
+#define BRCMF_DMI_SAFE_CHAR '_'
+
struct brcmf_dmi_data {
u32 chip;
u32 chiprev;
@@ -99,6 +107,15 @@ static const struct dmi_system_id dmi_platform_data[] = {
{}
};
+void brcmf_dmi_sanitize(char *dst, const unsigned char *allowed, char safe)
+{
+ while (*dst) {
+ if ((*dst < 0) || !(allowed[*dst / 8] & (1 << (*dst % 8))))
+ *dst = safe;
+ dst++;
+ }
+}
+
void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev)
{
const struct dmi_system_id *match;
@@ -126,6 +143,9 @@ void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev)
if (sys_vendor && product_name) {
snprintf(dmi_board_type, sizeof(dmi_board_type), "%s-%s",
sys_vendor, product_name);
+ brcmf_dmi_sanitize(dmi_board_type,
+ brcmf_dmi_allowed_chars,
+ BRCMF_DMI_SAFE_CHAR);
settings->board_type = dmi_board_type;
}
}
^ permalink raw reply related
* Re: [PATCH RFC] brcmfmac: sanitize DMI strings
From: Victor Bravo @ 2019-05-05 14:52 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Kalle Valo, David S. Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-kernel,
Hans de Goede
In-Reply-To: <20190505144852.addbdluel7edoevm@localhost>
Didn't notice that the patch removes some fresh changes which got in
while I was tuning it. Please wait for v2. Sorry for the noise.
v.
On Sun, May 05, 2019 at 04:48:52PM +0200, Victor Bravo wrote:
> Sanitize DMI strings in brcmfmac driver to make resulting filenames
> contain only safe characters. This version replaces all non-printable
> characters incl. delete (0-31, 127-255), spaces and slashes with
> underscores.
>
> This change breaks backward compatibility, but adds control over strings
> passed to firmware loader and compatibility with CONFIG_EXTRA_FIRMWARE
> which doesn't support spaces in filenames.
>
> Signed-off-by: Victor Bravo <1905@spmblk.com>
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
> index 7535cb0d4ac0..fa654ce7172b 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
> @@ -23,6 +23,14 @@
> /* The DMI data never changes so we can use a static buf for this */
> static char dmi_board_type[128];
>
> +/* Array of 128 bits representing 7-bit characters allowed in DMI strings. */
> +static unsigned char brcmf_dmi_allowed_chars[] = {
> + 0x00, 0x00, 0x00, 0x00, 0xfe, 0x7f, 0xff, 0xff,
> + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f
> +};
> +
> +#define BRCMF_DMI_SAFE_CHAR '_'
> +
> struct brcmf_dmi_data {
> u32 chip;
> u32 chiprev;
> @@ -43,10 +51,6 @@ static const struct brcmf_dmi_data meegopad_t08_data = {
> BRCM_CC_43340_CHIP_ID, 2, "meegopad-t08"
> };
>
> -static const struct brcmf_dmi_data pov_tab_p1006w_data = {
> - BRCM_CC_43340_CHIP_ID, 2, "pov-tab-p1006w-data"
> -};
> -
> static const struct dmi_system_id dmi_platform_data[] = {
> {
> /* Match for the GPDwin which unfortunately uses somewhat
> @@ -85,20 +89,18 @@ static const struct dmi_system_id dmi_platform_data[] = {
> },
> .driver_data = (void *)&meegopad_t08_data,
> },
> - {
> - /* Point of View TAB-P1006W-232 */
> - .matches = {
> - DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Insyde"),
> - DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "BayTrail"),
> - /* Note 105b is Foxcon's USB/PCI vendor id */
> - DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "105B"),
> - DMI_EXACT_MATCH(DMI_BOARD_NAME, "0E57"),
> - },
> - .driver_data = (void *)&pov_tab_p1006w_data,
> - },
> {}
> };
>
> +void brcmf_dmi_sanitize(char *dst, const unsigned char *allowed, char safe)
> +{
> + while (*dst) {
> + if ((*dst < 0) || !(allowed[*dst / 8] & (1 << (*dst % 8))))
> + *dst = safe;
> + dst++;
> + }
> +}
> +
> void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev)
> {
> const struct dmi_system_id *match;
> @@ -126,6 +128,9 @@ void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev)
> if (sys_vendor && product_name) {
> snprintf(dmi_board_type, sizeof(dmi_board_type), "%s-%s",
> sys_vendor, product_name);
> + brcmf_dmi_sanitize(dmi_board_type,
> + brcmf_dmi_allowed_chars,
> + BRCMF_DMI_SAFE_CHAR);
> settings->board_type = dmi_board_type;
> }
> }
>
--
NOTE FOR WINDOWS (TM) USERS: IN NO EVENT UNLESS REQUIRED BY APPLICABLE
LAW WILL I BE LIABLE TO YOU FOR ANY SW OR HW DAMAGE, SYSTEM MALFUNCTION,
DATA LOSS AND/OR DATA BREACH ARISING OUT WHILE YOU ARE READING THIS NOTE.
^ permalink raw reply
* [PATCH RFC] brcmfmac: sanitize DMI strings
From: Victor Bravo @ 2019-05-05 14:48 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Kalle Valo, David S. Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-kernel,
Hans de Goede
In-Reply-To: <16a87149068.2764.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>
Sanitize DMI strings in brcmfmac driver to make resulting filenames
contain only safe characters. This version replaces all non-printable
characters incl. delete (0-31, 127-255), spaces and slashes with
underscores.
This change breaks backward compatibility, but adds control over strings
passed to firmware loader and compatibility with CONFIG_EXTRA_FIRMWARE
which doesn't support spaces in filenames.
Signed-off-by: Victor Bravo <1905@spmblk.com>
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
index 7535cb0d4ac0..fa654ce7172b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
@@ -23,6 +23,14 @@
/* The DMI data never changes so we can use a static buf for this */
static char dmi_board_type[128];
+/* Array of 128 bits representing 7-bit characters allowed in DMI strings. */
+static unsigned char brcmf_dmi_allowed_chars[] = {
+ 0x00, 0x00, 0x00, 0x00, 0xfe, 0x7f, 0xff, 0xff,
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f
+};
+
+#define BRCMF_DMI_SAFE_CHAR '_'
+
struct brcmf_dmi_data {
u32 chip;
u32 chiprev;
@@ -43,10 +51,6 @@ static const struct brcmf_dmi_data meegopad_t08_data = {
BRCM_CC_43340_CHIP_ID, 2, "meegopad-t08"
};
-static const struct brcmf_dmi_data pov_tab_p1006w_data = {
- BRCM_CC_43340_CHIP_ID, 2, "pov-tab-p1006w-data"
-};
-
static const struct dmi_system_id dmi_platform_data[] = {
{
/* Match for the GPDwin which unfortunately uses somewhat
@@ -85,20 +89,18 @@ static const struct dmi_system_id dmi_platform_data[] = {
},
.driver_data = (void *)&meegopad_t08_data,
},
- {
- /* Point of View TAB-P1006W-232 */
- .matches = {
- DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Insyde"),
- DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "BayTrail"),
- /* Note 105b is Foxcon's USB/PCI vendor id */
- DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "105B"),
- DMI_EXACT_MATCH(DMI_BOARD_NAME, "0E57"),
- },
- .driver_data = (void *)&pov_tab_p1006w_data,
- },
{}
};
+void brcmf_dmi_sanitize(char *dst, const unsigned char *allowed, char safe)
+{
+ while (*dst) {
+ if ((*dst < 0) || !(allowed[*dst / 8] & (1 << (*dst % 8))))
+ *dst = safe;
+ dst++;
+ }
+}
+
void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev)
{
const struct dmi_system_id *match;
@@ -126,6 +128,9 @@ void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev)
if (sys_vendor && product_name) {
snprintf(dmi_board_type, sizeof(dmi_board_type), "%s-%s",
sys_vendor, product_name);
+ brcmf_dmi_sanitize(dmi_board_type,
+ brcmf_dmi_allowed_chars,
+ BRCMF_DMI_SAFE_CHAR);
settings->board_type = dmi_board_type;
}
}
^ permalink raw reply related
* Re: PROBLEM: brcmfmac's DMI-based fw file names break built-in fw loader
From: Victor Bravo @ 2019-05-05 14:36 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Kalle Valo, David S. Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-kernel,
Hans de Goede
In-Reply-To: <16a87149068.2764.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>
On Sun, May 05, 2019 at 10:20:33AM +0200, Arend Van Spriel wrote:
> On May 4, 2019 9:44:51 PM Victor Bravo <1905@spmblk.com> wrote:
>
> > On Sat, May 04, 2019 at 09:11:09PM +0200, Arend Van Spriel wrote:
> > > + Hans, Luis
> > >
> > > On 5/4/2019 6:26 PM, Victor Bravo wrote:
> > > > The brcmfmac driver seems to have partially fixed problems which
> > > > prevented it to be used in shared system/kernel images for multiple
> > > > hardware by trying to load it's <config>.txt as
> > > > <config>.<dmi_sys_vendor>.<dmi_product_name>.txt first and then
> > > > falling back to <config>.txt. Real-life example:
> > > >
> > > > brcmfmac mmc1:0001:1: Direct firmware load for
> > > brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt failed
> > > with
> > > > error -2
> > > > brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43340-sdio for chip
> > > > BCM43340/2
> > > >
> > > > Unfortunately this doesn't really help on systems which use static
> > > > kernel with firmware blobs (and also text configuration files in case of
> > > > brcmfmac) built-in using CONFIG_EXTRA_FIRMWARE, as CONFIG_EXTRA_FIRMWARE
> > > > doesn't support spaces in file names - kernel build fails with
> > > >
> > > > CONFIG_EXTRA_FIRMWARE="brcm/brcmfmac43340-sdio.bin
> > > brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt"
> > > >
> > > > for obvious reasons. So the only way here is to stay with good old
> > > > brcmfmac43340-sdio.txt and support at most one brcmfmac-equipped machine
> > > > per kernel image.
> > > >
> > > > Please consider filtering the DMI strings and replacing spaces and
> > > > possibly other invalid characters with underscores, and/or adding module
> > > > parameter to allow passing the string from command line (using
> > > > brcmfmac.tag=t100 or brcmfmac.board=t100 to make the module load
> > > > brcmfmac43340-sdio.t100.txt seems nicer to me, and isn't prone to
> > > > breaking when DMI strings change on BIOS update).
> > >
> > > The intent of the DMI approach was to avoid end-users from passing module
> > > parameters for this. As to fixing DMI string usage patches are welcome.
> >
> > Well I think I could also provide a patch to fix, this can be easily
> > done by adding a string of allowed characters and then replacing
> > unknown ones with underscores.
> >
> > > > My brief grep-based research also suggest that strings retrieved
> > > > by dmi_get_system_info() are passed to firmware loader without any
> > > > checks for special character, /../ etc. I'm not sure whether this is
> > > > considered to be proper & safe use, but if it's not, it may also have
> > > > some security implications, as it allows attacker with access to DMI
> > > > strings (using root rights/other OS/BIOS/physical access) to mess
> > > > with kernel space or secure boot.
> > >
> > > Hmm. Attackers with that kind of access can do bad is a gazillion ways.
> >
> > Agreed. It will be definitely easier to make filenames contain only safe
> > characters than to discuss those ways.
> >
> > > > I would also really appreciate not allowing future brcm (and other)
> > > > drivers to leave staging area before they fully support =y.
> > >
> > > Define fully support. At the time we moved into the wireless tree (almost a
> > > decade ago) we did support =y. As such you could consider the DMI approach a
> > > regression, but I find that a bit harsh to say. Hans made a honest attempt
> > > and it is something that can be fixed. It can be you providing just that ;-)
> >
> > Well... I agree that the idea wasn't really complete ;).
> >
> > As for the patches, I also realized that the txt config file actually
> > comes from EFI/BIOS, so it's quite possible that it may differ between
> > BIOS versions. So I'm thinking of 3 patches here:
> >
> > 1) Character filtering as described above.
> >
> > 2) Adding bios_version next to board_type, and changing load order to
> >
> > <config>.<dmi_sys_vendor>.<dmi_product_name>.<dmi_bios_version>.txt
> > <config>.<dmi_sys_vendor>.<dmi_product_name>.txt
> > <config>.txt
> >
> > 3) Adding command-line parameters to override these on problems.
> >
> > 1) breaks backward compatibility, but the DMI code seems to be quite
> > new so hopefully many people don't rely on it yet.
> >
> > 2) & 3) are backward compatible.
>
> Actually, the configuration file, or nvram file as we tend to call it, does
> not come from EFI/BIOS. There are a few platforms that have the nvram file
> stored in EFI and it's name is well-defined. It does assume there is only
> one brcmfmac device in the system.
Good to know. Sounds like working nvram file should never need update
and changes related to BIOS version are not needed.
I've meanwhile prepared patch for character filtering, will post in
new thread w/ better subject.
Regards,
v.
^ permalink raw reply
* Re: PROBLEM: brcmfmac's DMI-based fw file names break built-in fw loader
From: Arend Van Spriel @ 2019-05-05 8:20 UTC (permalink / raw)
To: Victor Bravo
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Kalle Valo, David S. Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-kernel,
Hans de Goede
In-Reply-To: <20190504194440.4zcxjrtj2aft3ka4@localhost>
On May 4, 2019 9:44:51 PM Victor Bravo <1905@spmblk.com> wrote:
> On Sat, May 04, 2019 at 09:11:09PM +0200, Arend Van Spriel wrote:
>> + Hans, Luis
>>
>> On 5/4/2019 6:26 PM, Victor Bravo wrote:
>> > The brcmfmac driver seems to have partially fixed problems which
>> > prevented it to be used in shared system/kernel images for multiple
>> > hardware by trying to load it's <config>.txt as
>> > <config>.<dmi_sys_vendor>.<dmi_product_name>.txt first and then
>> > falling back to <config>.txt. Real-life example:
>> >
>> > brcmfmac mmc1:0001:1: Direct firmware load for
>> brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt failed with
>> > error -2
>> > brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43340-sdio for chip
>> > BCM43340/2
>> >
>> > Unfortunately this doesn't really help on systems which use static
>> > kernel with firmware blobs (and also text configuration files in case of
>> > brcmfmac) built-in using CONFIG_EXTRA_FIRMWARE, as CONFIG_EXTRA_FIRMWARE
>> > doesn't support spaces in file names - kernel build fails with
>> >
>> > CONFIG_EXTRA_FIRMWARE="brcm/brcmfmac43340-sdio.bin
>> brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt"
>> >
>> > for obvious reasons. So the only way here is to stay with good old
>> > brcmfmac43340-sdio.txt and support at most one brcmfmac-equipped machine
>> > per kernel image.
>> >
>> > Please consider filtering the DMI strings and replacing spaces and
>> > possibly other invalid characters with underscores, and/or adding module
>> > parameter to allow passing the string from command line (using
>> > brcmfmac.tag=t100 or brcmfmac.board=t100 to make the module load
>> > brcmfmac43340-sdio.t100.txt seems nicer to me, and isn't prone to
>> > breaking when DMI strings change on BIOS update).
>>
>> The intent of the DMI approach was to avoid end-users from passing module
>> parameters for this. As to fixing DMI string usage patches are welcome.
>
> Well I think I could also provide a patch to fix, this can be easily
> done by adding a string of allowed characters and then replacing
> unknown ones with underscores.
>
>> > My brief grep-based research also suggest that strings retrieved
>> > by dmi_get_system_info() are passed to firmware loader without any
>> > checks for special character, /../ etc. I'm not sure whether this is
>> > considered to be proper & safe use, but if it's not, it may also have
>> > some security implications, as it allows attacker with access to DMI
>> > strings (using root rights/other OS/BIOS/physical access) to mess
>> > with kernel space or secure boot.
>>
>> Hmm. Attackers with that kind of access can do bad is a gazillion ways.
>
> Agreed. It will be definitely easier to make filenames contain only safe
> characters than to discuss those ways.
>
>> > I would also really appreciate not allowing future brcm (and other)
>> > drivers to leave staging area before they fully support =y.
>>
>> Define fully support. At the time we moved into the wireless tree (almost a
>> decade ago) we did support =y. As such you could consider the DMI approach a
>> regression, but I find that a bit harsh to say. Hans made a honest attempt
>> and it is something that can be fixed. It can be you providing just that ;-)
>
> Well... I agree that the idea wasn't really complete ;).
>
> As for the patches, I also realized that the txt config file actually
> comes from EFI/BIOS, so it's quite possible that it may differ between
> BIOS versions. So I'm thinking of 3 patches here:
>
> 1) Character filtering as described above.
>
> 2) Adding bios_version next to board_type, and changing load order to
>
> <config>.<dmi_sys_vendor>.<dmi_product_name>.<dmi_bios_version>.txt
> <config>.<dmi_sys_vendor>.<dmi_product_name>.txt
> <config>.txt
>
> 3) Adding command-line parameters to override these on problems.
>
> 1) breaks backward compatibility, but the DMI code seems to be quite
> new so hopefully many people don't rely on it yet.
>
> 2) & 3) are backward compatible.
Actually, the configuration file, or nvram file as we tend to call it, does
not come from EFI/BIOS. There are a few platforms that have the nvram file
stored in EFI and it's name is well-defined. It does assume there is only
one brcmfmac device in the system.
Regards,
Arend
^ permalink raw reply
* Re: pull-request: wireless-drivers 2019-04-30
From: David Miller @ 2019-05-05 7:51 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87r29jo2jy.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Tue, 30 Apr 2019 19:55:45 +0300
> David Miller <davem@davemloft.net> writes:
>
>> Thanks for the conflict resolution information, it is very helpful.
>>
>> However, can you put it into the merge commit text next time as well?
>> I cut and pasted it in there when I pulled this stuff in.
>
> A good idea, I'll do that. Just to be sure, do you mean that I should
> add it only with conflicts between net and net-next (like in this case)?
> Or should I add it everytime I see a conflict, for example between
> wireless-drivers-next and net-next? I hope my question is not too
> confusing...
When there is a major conflict for me to resolve when I pull in your
pull reqeust, please place the conflict resolution help text into the
merge commit message.
I hope this is now clear :-)
^ permalink raw reply
* [PATCH] mt76: mt7603: stop mac80211 queues before setting the channel
From: Lorenzo Bianconi @ 2019-05-05 0:53 UTC (permalink / raw)
To: nbd; +Cc: lorenzo.bianconi, linux-wireless
In-Reply-To: <cover.1557016660.git.lorenzo@kernel.org>
Suspend data transmission during channel switch
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/net/wireless/mediatek/mt76/mt7603/main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/main.c b/drivers/net/wireless/mediatek/mt76/mt7603/main.c
index 0a0334dc40d5..172687a1cb15 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/main.c
@@ -207,8 +207,11 @@ mt7603_config(struct ieee80211_hw *hw, u32 changed)
int ret = 0;
if (changed & (IEEE80211_CONF_CHANGE_CHANNEL |
- IEEE80211_CONF_CHANGE_POWER))
+ IEEE80211_CONF_CHANGE_POWER)) {
+ ieee80211_stop_queues(hw);
ret = mt7603_set_channel(dev, &hw->conf.chandef);
+ ieee80211_wake_queues(hw);
+ }
if (changed & IEEE80211_CONF_CHANGE_MONITOR) {
mutex_lock(&dev->mt76.mutex);
--
2.20.1
^ permalink raw reply related
* static analysis issue in rtl8188de driver
From: Colin Ian King @ 2019-05-04 21:50 UTC (permalink / raw)
To: Ping-Ke Shih, Kalle Valo, Larry Finger, linux-wireless
Cc: linux-kernel@vger.kernel.org
Hi,
Static analysis with Coverity has found an issue in the rtl8188de
wireless driver in drivers/net/wireless/realtek/rtlwifi/rtl8192de/dm.c
in function tl92d_dm_txpower_tracking_callback_thermalmeter.
The issue is that u8 array ofdm_index[3] is never initialized, however
it is decremented and incremented in two places resulting in garbage
value from the stack being updated in the following code:
if (thermalvalue > rtlpriv->dm.thermalvalue) {
for (i = 0; i < rf; i++)
ofdm_index[i] -= delta;
cck_index -= delta;
} else {
for (i = 0; i < rf; i++)
ofdm_index[i] += index;
cck_index += index;
}
At my first look at the code I believe ofdm_index should be just
zero-initialized at declaration time, but I suspect that I'm overlooking
something maybe a bit deeper. Any ideas?
Colin
^ permalink raw reply
* Re: PROBLEM: brcmfmac's DMI-based fw file names break built-in fw loader
From: Victor Bravo @ 2019-05-04 19:44 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Kalle Valo, David S. Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-kernel,
Hans de Goede
In-Reply-To: <cce7604e-2b02-80ed-1df5-6f304cada0cb@broadcom.com>
On Sat, May 04, 2019 at 09:11:09PM +0200, Arend Van Spriel wrote:
> + Hans, Luis
>
> On 5/4/2019 6:26 PM, Victor Bravo wrote:
> > The brcmfmac driver seems to have partially fixed problems which
> > prevented it to be used in shared system/kernel images for multiple
> > hardware by trying to load it's <config>.txt as
> > <config>.<dmi_sys_vendor>.<dmi_product_name>.txt first and then
> > falling back to <config>.txt. Real-life example:
> >
> > brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt failed with
> > error -2
> > brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43340-sdio for chip
> > BCM43340/2
> >
> > Unfortunately this doesn't really help on systems which use static
> > kernel with firmware blobs (and also text configuration files in case of
> > brcmfmac) built-in using CONFIG_EXTRA_FIRMWARE, as CONFIG_EXTRA_FIRMWARE
> > doesn't support spaces in file names - kernel build fails with
> >
> > CONFIG_EXTRA_FIRMWARE="brcm/brcmfmac43340-sdio.bin brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt"
> >
> > for obvious reasons. So the only way here is to stay with good old
> > brcmfmac43340-sdio.txt and support at most one brcmfmac-equipped machine
> > per kernel image.
> >
> > Please consider filtering the DMI strings and replacing spaces and
> > possibly other invalid characters with underscores, and/or adding module
> > parameter to allow passing the string from command line (using
> > brcmfmac.tag=t100 or brcmfmac.board=t100 to make the module load
> > brcmfmac43340-sdio.t100.txt seems nicer to me, and isn't prone to
> > breaking when DMI strings change on BIOS update).
>
> The intent of the DMI approach was to avoid end-users from passing module
> parameters for this. As to fixing DMI string usage patches are welcome.
Well I think I could also provide a patch to fix, this can be easily
done by adding a string of allowed characters and then replacing
unknown ones with underscores.
> > My brief grep-based research also suggest that strings retrieved
> > by dmi_get_system_info() are passed to firmware loader without any
> > checks for special character, /../ etc. I'm not sure whether this is
> > considered to be proper & safe use, but if it's not, it may also have
> > some security implications, as it allows attacker with access to DMI
> > strings (using root rights/other OS/BIOS/physical access) to mess
> > with kernel space or secure boot.
>
> Hmm. Attackers with that kind of access can do bad is a gazillion ways.
Agreed. It will be definitely easier to make filenames contain only safe
characters than to discuss those ways.
> > I would also really appreciate not allowing future brcm (and other)
> > drivers to leave staging area before they fully support =y.
>
> Define fully support. At the time we moved into the wireless tree (almost a
> decade ago) we did support =y. As such you could consider the DMI approach a
> regression, but I find that a bit harsh to say. Hans made a honest attempt
> and it is something that can be fixed. It can be you providing just that ;-)
Well... I agree that the idea wasn't really complete ;).
As for the patches, I also realized that the txt config file actually
comes from EFI/BIOS, so it's quite possible that it may differ between
BIOS versions. So I'm thinking of 3 patches here:
1) Character filtering as described above.
2) Adding bios_version next to board_type, and changing load order to
<config>.<dmi_sys_vendor>.<dmi_product_name>.<dmi_bios_version>.txt
<config>.<dmi_sys_vendor>.<dmi_product_name>.txt
<config>.txt
3) Adding command-line parameters to override these on problems.
1) breaks backward compatibility, but the DMI code seems to be quite
new so hopefully many people don't rely on it yet.
2) & 3) are backward compatible.
Regards,
v.
^ permalink raw reply
* [PATCH] wireless-regdb: update source of information for ES
From: Xose Vazquez Perez @ 2019-05-04 19:18 UTC (permalink / raw)
Cc: Xose Vazquez Perez, Seth Forshee, WIRELESS ML, REGDB ML
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: WIRELESS ML <linux-wireless@vger.kernel.org>
Cc: REGDB ML <wireless-regdb@lists.infradead.org>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
---
db.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/db.txt b/db.txt
index 4fb1948..fe909a8 100644
--- a/db.txt
+++ b/db.txt
@@ -432,7 +432,7 @@ country EG: DFS-ETSI
# Source:
# Cuadro nacional de atribución de frecuencias (CNAF)
-# http://www.mincotur.gob.es/telecomunicaciones/espectro/paginas/cnaf.aspx
+# https://avancedigital.gob.es/espectro/Paginas/cnaf.aspx
country ES: DFS-ETSI
(2400 - 2483.5 @ 40), (100 mW)
(5150 - 5250 @ 80), (200 mW), NO-OUTDOOR, AUTO-BW, wmmrule=ETSI
--
2.21.0
^ permalink raw reply related
* Re: PROBLEM: brcmfmac's DMI-based fw file names break built-in fw loader
From: Arend Van Spriel @ 2019-05-04 19:11 UTC (permalink / raw)
To: Victor Bravo, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Kalle Valo, David S. Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-kernel,
Hans de Goede
In-Reply-To: <20190504162633.ldrz2nqfocg55grb@localhost>
+ Hans, Luis
On 5/4/2019 6:26 PM, Victor Bravo wrote:
> The brcmfmac driver seems to have partially fixed problems which
> prevented it to be used in shared system/kernel images for multiple
> hardware by trying to load it's <config>.txt as
> <config>.<dmi_sys_vendor>.<dmi_product_name>.txt first and then
> falling back to <config>.txt. Real-life example:
>
> brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt failed with
> error -2
> brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43340-sdio for chip
> BCM43340/2
>
> Unfortunately this doesn't really help on systems which use static
> kernel with firmware blobs (and also text configuration files in case of
> brcmfmac) built-in using CONFIG_EXTRA_FIRMWARE, as CONFIG_EXTRA_FIRMWARE
> doesn't support spaces in file names - kernel build fails with
>
> CONFIG_EXTRA_FIRMWARE="brcm/brcmfmac43340-sdio.bin brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt"
>
> for obvious reasons. So the only way here is to stay with good old
> brcmfmac43340-sdio.txt and support at most one brcmfmac-equipped machine
> per kernel image.
>
> Please consider filtering the DMI strings and replacing spaces and
> possibly other invalid characters with underscores, and/or adding module
> parameter to allow passing the string from command line (using
> brcmfmac.tag=t100 or brcmfmac.board=t100 to make the module load
> brcmfmac43340-sdio.t100.txt seems nicer to me, and isn't prone to
> breaking when DMI strings change on BIOS update).
The intent of the DMI approach was to avoid end-users from passing
module parameters for this. As to fixing DMI string usage patches are
welcome.
> My brief grep-based research also suggest that strings retrieved
> by dmi_get_system_info() are passed to firmware loader without any
> checks for special character, /../ etc. I'm not sure whether this is
> considered to be proper & safe use, but if it's not, it may also have
> some security implications, as it allows attacker with access to DMI
> strings (using root rights/other OS/BIOS/physical access) to mess
> with kernel space or secure boot.
Hmm. Attackers with that kind of access can do bad is a gazillion ways.
> I would also really appreciate not allowing future brcm (and other)
> drivers to leave staging area before they fully support =y.
Define fully support. At the time we moved into the wireless tree
(almost a decade ago) we did support =y. As such you could consider the
DMI approach a regression, but I find that a bit harsh to say. Hans made
a honest attempt and it is something that can be fixed. It can be you
providing just that ;-)
Regards,
Arend
^ permalink raw reply
* PROBLEM: brcmfmac's DMI-based fw file names break built-in fw loader
From: Victor Bravo @ 2019-05-04 16:26 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Kalle Valo, David S. Miller, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, linux-kernel
The brcmfmac driver seems to have partially fixed problems which
prevented it to be used in shared system/kernel images for multiple
hardware by trying to load it's <config>.txt as
<config>.<dmi_sys_vendor>.<dmi_product_name>.txt first and then
falling back to <config>.txt. Real-life example:
brcmfmac mmc1:0001:1: Direct firmware load for brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt failed with
error -2
brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43340-sdio for chip
BCM43340/2
Unfortunately this doesn't really help on systems which use static
kernel with firmware blobs (and also text configuration files in case of
brcmfmac) built-in using CONFIG_EXTRA_FIRMWARE, as CONFIG_EXTRA_FIRMWARE
doesn't support spaces in file names - kernel build fails with
CONFIG_EXTRA_FIRMWARE="brcm/brcmfmac43340-sdio.bin brcm/brcmfmac43340-sdio.ASUSTeK COMPUTER INC.-T100HAN.txt"
for obvious reasons. So the only way here is to stay with good old
brcmfmac43340-sdio.txt and support at most one brcmfmac-equipped machine
per kernel image.
Please consider filtering the DMI strings and replacing spaces and
possibly other invalid characters with underscores, and/or adding module
parameter to allow passing the string from command line (using
brcmfmac.tag=t100 or brcmfmac.board=t100 to make the module load
brcmfmac43340-sdio.t100.txt seems nicer to me, and isn't prone to
breaking when DMI strings change on BIOS update).
My brief grep-based research also suggest that strings retrieved
by dmi_get_system_info() are passed to firmware loader without any
checks for special character, /../ etc. I'm not sure whether this is
considered to be proper & safe use, but if it's not, it may also have
some security implications, as it allows attacker with access to DMI
strings (using root rights/other OS/BIOS/physical access) to mess
with kernel space or secure boot.
I would also really appreciate not allowing future brcm (and other)
drivers to leave staging area before they fully support =y.
Regards,
v.b.
^ permalink raw reply
* [PATCH 17/17] mt7615: mcu: run __mt76_mcu_send_msg in mt7615_mcu_send_firmware
From: Lorenzo Bianconi @ 2019-05-04 15:29 UTC (permalink / raw)
To: nbd; +Cc: lorenzo.bianconi, linux-wireless, ryder.lee, royluo
In-Reply-To: <cover.1556981521.git.lorenzo@kernel.org>
Run __mt76_mcu_send_msg instead of __mt7615_mcu_msg_send and remove
duplicated code.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index 4fa41e2c5f77..5b01abe7aecf 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -179,19 +179,14 @@ static int mt7615_mcu_init_download(struct mt7615_dev *dev, u32 addr,
static int mt7615_mcu_send_firmware(struct mt7615_dev *dev, const void *data,
int len)
{
- struct sk_buff *skb;
- int ret = 0;
+ int ret = 0, cur_len;
while (len > 0) {
- int cur_len = min_t(int, 4096 - sizeof(struct mt7615_mcu_txd),
- len);
-
- skb = mt7615_mcu_msg_alloc(data, cur_len);
- if (!skb)
- return -ENOMEM;
+ cur_len = min_t(int, 4096 - sizeof(struct mt7615_mcu_txd),
+ len);
- ret = __mt7615_mcu_msg_send(dev, skb, -MCU_CMD_FW_SCATTER,
- NULL);
+ ret = __mt76_mcu_send_msg(&dev->mt76, -MCU_CMD_FW_SCATTER,
+ data, cur_len, false);
if (ret)
break;
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox