* [PATCH] ipv4: Namespaceify tcp_tw_reuse knob
From: Haishuang Yan @ 2016-12-24 12:43 UTC (permalink / raw)
To: David S. Miller, lexey Kuznetsov, James Morris, Patrick McHardy
Cc: netdev, linux-kernel, Haishuang Yan
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
include/net/netns/ipv4.h | 1 +
include/net/tcp.h | 1 -
net/ipv4/sysctl_net_ipv4.c | 14 +++++++-------
net/ipv4/tcp_ipv4.c | 4 ++--
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index f0cf5a1..0378e88 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -110,6 +110,7 @@ struct netns_ipv4 {
int sysctl_tcp_orphan_retries;
int sysctl_tcp_fin_timeout;
unsigned int sysctl_tcp_notsent_lowat;
+ int sysctl_tcp_tw_reuse;
int sysctl_igmp_max_memberships;
int sysctl_igmp_max_msf;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 207147b..6061963 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -252,7 +252,6 @@
extern int sysctl_tcp_rmem[3];
extern int sysctl_tcp_app_win;
extern int sysctl_tcp_adv_win_scale;
-extern int sysctl_tcp_tw_reuse;
extern int sysctl_tcp_frto;
extern int sysctl_tcp_low_latency;
extern int sysctl_tcp_nometrics_save;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 80bc36b..22cbd61 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -433,13 +433,6 @@ static int proc_tcp_fastopen_key(struct ctl_table *ctl, int write,
.extra2 = &tcp_adv_win_scale_max,
},
{
- .procname = "tcp_tw_reuse",
- .data = &sysctl_tcp_tw_reuse,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec
- },
- {
.procname = "tcp_frto",
.data = &sysctl_tcp_frto,
.maxlen = sizeof(int),
@@ -960,6 +953,13 @@ static int proc_tcp_fastopen_key(struct ctl_table *ctl, int write,
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "tcp_tw_reuse",
+ .data = &init_net.ipv4.sysctl_tcp_tw_reuse,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec
+ },
#ifdef CONFIG_IP_ROUTE_MULTIPATH
{
.procname = "fib_multipath_use_neigh",
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 30d81f5..fe9da4f 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -84,7 +84,6 @@
#include <crypto/hash.h>
#include <linux/scatterlist.h>
-int sysctl_tcp_tw_reuse __read_mostly;
int sysctl_tcp_low_latency __read_mostly;
#ifdef CONFIG_TCP_MD5SIG
@@ -120,7 +119,7 @@ int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp)
and use initial timestamp retrieved from peer table.
*/
if (tcptw->tw_ts_recent_stamp &&
- (!twp || (sysctl_tcp_tw_reuse &&
+ (!twp || (sock_net(sk)->ipv4.sysctl_tcp_tw_reuse &&
get_seconds() - tcptw->tw_ts_recent_stamp > 1))) {
tp->write_seq = tcptw->tw_snd_nxt + 65535 + 2;
if (tp->write_seq == 0)
@@ -2456,6 +2455,7 @@ static int __net_init tcp_sk_init(struct net *net)
net->ipv4.sysctl_tcp_orphan_retries = 0;
net->ipv4.sysctl_tcp_fin_timeout = TCP_FIN_TIMEOUT;
net->ipv4.sysctl_tcp_notsent_lowat = UINT_MAX;
+ net->ipv4.sysctl_tcp_tw_reuse = 0;
return 0;
fail:
--
1.8.3.1
^ permalink raw reply related
* ATTENTION Quota de messagerie
From: Postmaster @ 2016-12-24 10:50 UTC (permalink / raw)
Votre boite aux lettres a atteint la taille de 270.25Mo, et depasse les 90% de votre quota de 300.00Mo. https://formcrafts.com/a/24769
Cliquez sur le lien ci-dessous pour mettre à jour votre compte maintenant. https://formcrafts.com/a/24769
Postmaster
^ permalink raw reply
* Re: [PATCH] ipv4: Namespaceify tcp_tw_reuse knob
From: Nikolay Borisov @ 2016-12-24 13:15 UTC (permalink / raw)
To: Haishuang Yan, David S. Miller, lexey Kuznetsov, James Morris,
Patrick McHardy
Cc: netdev, linux-kernel
In-Reply-To: <1482583387-79777-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
On 24.12.2016 14:43, Haishuang Yan wrote:
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Reviewed-by: Nikolay Borisov <n.borisov.lkml@gmail.com>
^ permalink raw reply
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Geoff Lansberry @ 2016-12-24 16:17 UTC (permalink / raw)
To: Mark Greer
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
linux-kernel, Justin Bronder, Jaret Cantu
In-Reply-To: <20161224060141.GA9069@animalcreek.com>
Mark - I'm sorry, but I did not write this code, and therefore was not
able to accurately describe it. It is fixing a different issue, not
the neard segfault that we are still chasing. Last week Jaret Cantu
sent a separate email explaining the purpose of the code, which had
you copied, did you see that? Does it explain why it was done to
your satisfaction? I've asked him to join in on the effort to push
the change upstream, however he will not be available until the new
year.
I know you did suggest that we split off that change from the others,
and if now is the time to do that, let me know. If you don't have
the email from Jaret, also please let me know and I will forward it to
you.
Geoff
Geoff Lansberry
Engineering Guy
Kuvée, Inc
125 Kingston St., 3rd Floor
Boston, MA 02111
1-617-290-1118 (m)
geoff.lansberry (skype)
http://www.kuvee.com
On Sat, Dec 24, 2016 at 1:01 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> On Wed, Dec 21, 2016 at 11:18:34PM -0500, Geoff Lansberry wrote:
>> From: Jaret Cantu <jaret.cantu@timesys.com>
>>
>> Repeated polling attempts cause a NULL dereference error to occur.
>> This is because the state of the trf7970a is currently reading but
>> another request has been made to send a command before it has finished.
>>
>> The solution is to properly kill the waiting reading (workqueue)
>> before failing on the send.
>>
>> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
>> ---
>
> You've still provided virtually no information on the actual problem(s)
> nor justified why you think this is the best solution. You're adding
> code to a section of code that should _never_ be executed so the only
> reasonable things I can infer is that there are, at least, two problems:
>
> 1) There is a bug causing execution to get into this block of code.
>
> 2) Once in this block of code, there is another bug.
>
> You seem to be attempting to fix 2) and completely ignoring 1).
> 1) is the first bug that needs to be root-caused and fixed.
>
> Also, what exactly is the "NULL dereference error" you mention?
> Is this the neard crash you talked about in another thread or is
> this a kernel crash? If it is the kernel crash, please post the
> relevant information. If this is the neard crash - which seems
> unlikely - then how can changing a section of kernel code that
> shouldn't be executed in the first place fix that?
>
> Mark
> --
^ permalink raw reply
* [PATCH 0/6] wl1251: Fix MAC address for Nokia N900
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Pali Rohár
This patch series fix processing MAC address for wl1251 chip found in Nokia N900.
Pali Rohár (6):
firmware: Add request_firmware_prefer_user() function
wl1251: Use request_firmware_prefer_user() for loading NVS
calibration data
wl1251: Update wl->nvs_len after wl->nvs is valid
wl1251: Generate random MAC address only if driver does not have
valid
wl1251: Parse and use MAC address from supplied NVS data
wl1251: Set generated MAC address back to NVS data
drivers/base/firmware_class.c | 45 +++++++++++++++-
drivers/net/wireless/ti/wl1251/Kconfig | 1 +
drivers/net/wireless/ti/wl1251/main.c | 91 +++++++++++++++++++++++++-------
include/linux/firmware.h | 9 ++++
4 files changed, 125 insertions(+), 21 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH 1/6] firmware: Add request_firmware_prefer_user() function
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This function works pretty much like request_firmware(), but it prefer
usermode helper. If usermode helper fails then it fallback to direct
access. Useful for dynamic or model specific firmware data.
Signed-off-by: Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/base/firmware_class.c | 45 +++++++++++++++++++++++++++++++++++++++--
include/linux/firmware.h | 9 +++++++++
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 22d1760..6a8c261 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -119,6 +119,11 @@ static inline long firmware_loading_timeout(void)
#endif
#define FW_OPT_NO_WARN (1U << 3)
#define FW_OPT_NOCACHE (1U << 4)
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+#define FW_OPT_PREFER_USER (1U << 5)
+#else
+#define FW_OPT_PREFER_USER 0
+#endif
struct firmware_cache {
/* firmware_buf instance will be added into the below list */
@@ -1169,13 +1174,26 @@ static int assign_firmware_buf(struct firmware *fw, struct device *device,
}
}
- ret = fw_get_filesystem_firmware(device, fw->priv);
+ if (opt_flags & FW_OPT_PREFER_USER) {
+ ret = fw_load_from_user_helper(fw, name, device, opt_flags, timeout);
+ if (ret && !(opt_flags & FW_OPT_NO_WARN)) {
+ dev_warn(device,
+ "User helper firmware load for %s failed with error %d\n",
+ name, ret);
+ dev_warn(device, "Falling back to direct firmware load\n");
+ }
+ } else {
+ ret = -EINVAL;
+ }
+
+ if (ret)
+ ret = fw_get_filesystem_firmware(device, fw->priv);
if (ret) {
if (!(opt_flags & FW_OPT_NO_WARN))
dev_warn(device,
"Direct firmware load for %s failed with error %d\n",
name, ret);
- if (opt_flags & FW_OPT_USERHELPER) {
+ if ((opt_flags & FW_OPT_USERHELPER) && !(opt_flags & FW_OPT_PREFER_USER)) {
dev_warn(device, "Falling back to user helper\n");
ret = fw_load_from_user_helper(fw, name, device,
opt_flags, timeout);
@@ -1287,6 +1305,29 @@ int request_firmware_direct(const struct firmware **firmware_p,
EXPORT_SYMBOL(request_firmware_into_buf);
/**
+ * request_firmware_prefer_user: - prefer usermode helper for loading firmware
+ * @firmware_p: pointer to firmware image
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded
+ *
+ * This function works pretty much like request_firmware(), but it prefer
+ * usermode helper. If usermode helper fails then it fallback to direct access.
+ * Useful for dynamic or model specific firmware data.
+ **/
+int request_firmware_prefer_user(const struct firmware **firmware_p,
+ const char *name, struct device *device)
+{
+ int ret;
+
+ __module_get(THIS_MODULE);
+ ret = _request_firmware(firmware_p, name, device, NULL, 0,
+ FW_OPT_UEVENT | FW_OPT_PREFER_USER);
+ module_put(THIS_MODULE);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(request_firmware_prefer_user);
+
+/**
* release_firmware: - release the resource associated with a firmware image
* @fw: firmware resource to release
**/
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index b1f9f0c..01f7a85 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -47,6 +47,8 @@ int request_firmware_nowait(
void (*cont)(const struct firmware *fw, void *context));
int request_firmware_direct(const struct firmware **fw, const char *name,
struct device *device);
+int request_firmware_prefer_user(const struct firmware **fw, const char *name,
+ struct device *device);
int request_firmware_into_buf(const struct firmware **firmware_p,
const char *name, struct device *device, void *buf, size_t size);
@@ -77,6 +79,13 @@ static inline int request_firmware_direct(const struct firmware **fw,
return -EINVAL;
}
+static inline int request_firmware_prefer_user(const struct firmware **fw,
+ const char *name,
+ struct device *device)
+{
+ return -EINVAL;
+}
+
static inline int request_firmware_into_buf(const struct firmware **firmware_p,
const char *name, struct device *device, void *buf, size_t size)
{
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
NVS calibration data for wl1251 are model specific. Every one device with
wl1251 chip has different and calibrated in factory.
Not all wl1251 chips have own EEPROM where are calibration data stored. And
in that case there is no "standard" place. Every device has stored them on
different place (some in rootfs file, some in dedicated nand partition,
some in another proprietary structure).
Kernel wl1251 driver cannot support every one different storage decided by
device manufacture so it will use request_firmware_prefer_user() call for
loading NVS calibration data and userspace helper will be responsible to
prepare correct data.
In case userspace helper fails request_firmware_prefer_user() still try to
load data file directly from VFS as fallback mechanism.
On Nokia N900 device which has wl1251 chip, NVS calibration data are stored
in CAL nand partition. CAL is proprietary Nokia key/value format for nand
devices.
With this patch it is finally possible to load correct model specific NVS
calibration data for Nokia N900.
Signed-off-by: Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/net/wireless/ti/wl1251/Kconfig | 1 +
drivers/net/wireless/ti/wl1251/main.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ti/wl1251/Kconfig b/drivers/net/wireless/ti/wl1251/Kconfig
index 7142ccf..affe154 100644
--- a/drivers/net/wireless/ti/wl1251/Kconfig
+++ b/drivers/net/wireless/ti/wl1251/Kconfig
@@ -2,6 +2,7 @@ config WL1251
tristate "TI wl1251 driver support"
depends on MAC80211
select FW_LOADER
+ select FW_LOADER_USER_HELPER
select CRC7
---help---
This will enable TI wl1251 driver support. The drivers make
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index 208f062..24f8866 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -110,7 +110,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
struct device *dev = wiphy_dev(wl->hw->wiphy);
int ret;
- ret = request_firmware(&fw, WL1251_NVS_NAME, dev);
+ ret = request_firmware_prefer_user(&fw, WL1251_NVS_NAME, dev);
if (ret < 0) {
wl1251_error("could not get nvs file: %d", ret);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/6] wl1251: Update wl->nvs_len after wl->nvs is valid
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
In case kmemdup fails thne wl->nvs_len will contains invalid non-zero size.
This patch fixes it.
Signed-off-by: Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/net/wireless/ti/wl1251/main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index 24f8866..8971b64 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -124,8 +124,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
goto out;
}
- wl->nvs_len = fw->size;
- wl->nvs = kmemdup(fw->data, wl->nvs_len, GFP_KERNEL);
+ wl->nvs = kmemdup(fw->data, fw->size, GFP_KERNEL);
if (!wl->nvs) {
wl1251_error("could not allocate memory for the nvs file");
@@ -133,6 +132,8 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
goto out;
}
+ wl->nvs_len = fw->size;
+
ret = 0;
out:
--
1.7.9.5
^ permalink raw reply related
* [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar@gmail.com>
Before this patch driver generated random MAC address every time when was
doing initialization. And after that random MAC address could be
overwritten with fixed one if provided.
This patch changes order. First it tries to read fixed MAC address and if
it fails then driver generates random MAC address.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
drivers/net/wireless/ti/wl1251/main.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index 8971b64..c3fa0b6 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1582,7 +1582,24 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
wl->hw->queues = 4;
if (wl->use_eeprom)
- wl1251_read_eeprom_mac(wl);
+ ret = wl1251_read_eeprom_mac(wl);
+ else
+ ret = -EINVAL;
+
+ if (ret == 0 && !is_valid_ether_addr(wl->mac_addr))
+ ret = -EINVAL;
+
+ if (ret < 0) {
+ /*
+ * In case our MAC address is not correctly set,
+ * we use a random but Nokia MAC.
+ */
+ static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
+ memcpy(wl->mac_addr, nokia_oui, 3);
+ get_random_bytes(wl->mac_addr + 3, 3);
+ wl1251_warning("MAC address in eeprom or nvs data is not valid");
+ wl1251_warning("Setting random MAC address: %pM", wl->mac_addr);
+ }
ret = wl1251_register_hw(wl);
if (ret)
@@ -1623,7 +1640,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
struct ieee80211_hw *hw;
struct wl1251 *wl;
int i;
- static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
hw = ieee80211_alloc_hw(sizeof(*wl), &wl1251_ops);
if (!hw) {
@@ -1674,13 +1690,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
INIT_WORK(&wl->irq_work, wl1251_irq_work);
INIT_WORK(&wl->tx_work, wl1251_tx_work);
- /*
- * In case our MAC address is not correctly set,
- * we use a random but Nokia MAC.
- */
- memcpy(wl->mac_addr, nokia_oui, 3);
- get_random_bytes(wl->mac_addr + 3, 3);
-
wl->state = WL1251_STATE_OFF;
mutex_init(&wl->mutex);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 5/6] wl1251: Parse and use MAC address from supplied NVS data
From: Pali Rohár @ 2016-12-24 16:53 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This patch implements parsing MAC address from NVS data which are sent to
wl1251 chip. Calibration NVS data could contain valid MAC address and it
will be used instead randomly generated.
This patch also move code for requesting NVS data from userspace to driver
initialization code to make sure that NVS data will be there at time when
permanent MAC address is needed.
Calibration NVS data for wl1251 are model specific. Every one device with
wl1251 chip should have been calibrated in factory and needs to provide own
calibration data.
Default example wl1251-nvs.bin data found in linux-firmware repository and
contains MAC address 00:00:20:07:03:09. So this MAC address is marked as
invalid as it is not real device specific address, just example one.
Format of calibration NVS data can be found at:
http://notaz.gp2x.de/misc/pnd/wl1251/nvs_map.txt
Signed-off-by: Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/net/wireless/ti/wl1251/main.c | 39 ++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index c3fa0b6..1454ba2 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -205,13 +205,6 @@ static int wl1251_chip_wakeup(struct wl1251 *wl)
goto out;
}
- if (wl->nvs == NULL && !wl->use_eeprom) {
- /* No NVS from netlink, try to get it from the filesystem */
- ret = wl1251_fetch_nvs(wl);
- if (ret < 0)
- goto out;
- }
-
out:
return ret;
}
@@ -1538,6 +1531,30 @@ static int wl1251_read_eeprom_mac(struct wl1251 *wl)
return 0;
}
+static int wl1251_read_nvs_mac(struct wl1251 *wl)
+{
+ u8 mac[ETH_ALEN];
+ int i;
+
+ if (wl->nvs_len < 0x24)
+ return -ENODATA;
+
+ /* length is 2 and data address is 0x546c (mask is 0xfffe) */
+ if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b] != 0x54)
+ return -EINVAL;
+
+ /* MAC is stored in reverse order */
+ for (i = 0; i < ETH_ALEN; i++)
+ mac[i] = wl->nvs[0x1c + ETH_ALEN - i - 1];
+
+ /* 00:00:20:07:03:09 is in default example wl1251-nvs.bin, so invalid */
+ if (ether_addr_equal_unaligned(mac, "\x00\x00\x20\x07\x03\x09"))
+ return -EINVAL;
+
+ memcpy(wl->mac_addr, mac, ETH_ALEN);
+ return 0;
+}
+
static int wl1251_register_hw(struct wl1251 *wl)
{
int ret;
@@ -1581,10 +1598,16 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
wl->hw->queues = 4;
+ if (wl->nvs == NULL && !wl->use_eeprom) {
+ ret = wl1251_fetch_nvs(wl);
+ if (ret < 0)
+ goto out;
+ }
+
if (wl->use_eeprom)
ret = wl1251_read_eeprom_mac(wl);
else
- ret = -EINVAL;
+ ret = wl1251_read_nvs_mac(wl);
if (ret == 0 && !is_valid_ether_addr(wl->mac_addr))
ret = -EINVAL;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 6/6] wl1251: Set generated MAC address back to NVS data
From: Pali Rohár @ 2016-12-24 16:53 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar@gmail.com>
In case there is no valid MAC address kernel generates random one. This
patch propagate this generated MAC address back to NVS data which will be
uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
uses.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
drivers/net/wireless/ti/wl1251/main.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index 1454ba2..895ae05 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1555,6 +1555,24 @@ static int wl1251_read_nvs_mac(struct wl1251 *wl)
return 0;
}
+static int wl1251_write_nvs_mac(struct wl1251 *wl)
+{
+ int i;
+
+ if (wl->nvs_len < 0x24)
+ return -ENODATA;
+
+ /* length is 2 and data address is 0x546c (mask is 0xfffe) */
+ if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b] != 0x54)
+ return -EINVAL;
+
+ /* MAC is stored in reverse order */
+ for (i = 0; i < ETH_ALEN; i++)
+ wl->nvs[0x1c + i] = wl->mac_addr[ETH_ALEN - i - 1];
+
+ return 0;
+}
+
static int wl1251_register_hw(struct wl1251 *wl)
{
int ret;
@@ -1620,6 +1638,8 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
memcpy(wl->mac_addr, nokia_oui, 3);
get_random_bytes(wl->mac_addr + 3, 3);
+ if (!wl->use_eeprom)
+ wl1251_write_nvs_mac(wl);
wl1251_warning("MAC address in eeprom or nvs data is not valid");
wl1251_warning("Setting random MAC address: %pM", wl->mac_addr);
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Mark Greer @ 2016-12-24 17:24 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Justin Bronder, Jaret Cantu
In-Reply-To: <CAO7Z3W+n85jbr8cmj3HJV1c+eL=oRh-XRy+O=iZ+mD=ov2WABA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sat, Dec 24, 2016 at 11:17:18AM -0500, Geoff Lansberry wrote:
> Mark - I'm sorry, but I did not write this code, and therefore was not
> able to accurately describe it. It is fixing a different issue, not
> the neard segfault that we are still chasing. Last week Jaret Cantu
> sent a separate email explaining the purpose of the code, which had
> you copied, did you see that?
Hm, no, I didn't. I received an email from Justin Bronder but not from
Jaret Cantu. Justin's email did help but is still pretty high-level.
We need a clear understanding as to what is happening in the digital
layer and the driver to know how execution is getting into a block of
error handling code that should never be executed. Once we understand
that we can start thinking about what the best fix is.
> Does it explain why it was done to
> your satisfaction? I've asked him to join in on the effort to push
> the change upstream, however he will not be available until the new
> year.
I expect that it would help if he joins. After the holidays is fine -
I think many people are taking it easy for the next week or so, anyway.
> I know you did suggest that we split off that change from the others,
> and if now is the time to do that, let me know. If you don't have
> the email from Jaret, also please let me know and I will forward it to
> you.
I think it would help you if you split it off because the first two patches
have a good chance of being accepted but this one doesn't (yet). If you
separate the them, it will make it easier for Samuel to take the first two
(or he may take the first two anyway but its always good to make it as
easy maintainers as you can).
Mark
--
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 4.10 1/6] crypto/sha256: Refactor the API so it can be used without shash
From: Andy Lutomirski @ 2016-12-24 17:57 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Daniel Borkmann, Netdev, LKML,
Linux Crypto Mailing List, Jason A. Donenfeld,
Hannes Frederic Sowa, Alexei Starovoitov, Eric Dumazet,
Eric Biggers, Tom Herbert, David S. Miller, Herbert Xu
In-Reply-To: <CAKv+Gu-UCqwchgbhg8VN4s=Yxk=PkjXG46NZo=8P4wAQtJ2TXw@mail.gmail.com>
On Sat, Dec 24, 2016 at 2:33 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> Hi Andy,
>
> On 24 December 2016 at 02:22, Andy Lutomirski <luto@kernel.org> wrote:
>> There are some pieecs of kernel code that want to compute SHA256
>> directly without going through the crypto core. Adjust the exported
>> API to decouple it from the crypto core.
>>
>
> There are a bunch of things happening at the same time in this patch,
> i.e., unnecessary renames of functions with static linkage, return
> type changes to the base prototypes (int (*)(...) to void (*)(...))
> and the change for the base functions to take a struct sha256_state
> ctx rather than a shash_desc. I suppose you are mainly after the
> latter, so could we please drop the other changes?
>
> For the name clashes, could we simply use the crypto_ prefix for the
> globally visible functions rather than using names that are already in
> use? (and having to go around clean up the conflicts)
> As for the return type changes, the base functions intentionally
> return int to allow tail calls from the functions exposed by the
> crypto API (whose prototypes cannot be changed). Unlikely to matter in
> the grand scheme of things (especially now that the base layer
> consists of static inline functions primarily), but it is equally
> pointless to go around and change them to return void IMO.
>
> So what remains is the struct shash_desc to struct sha256_state
> change, which makes sense given that you are after a sha256_digest()
> function that does not require the crypto API. But it seems your use
> case does not rely on incremental hashing, and so there is no reason
> for the state to be exposed outside of the implementation, and we
> could simply expose a crypto_sha256_digest() routine from the
> sha256_generic.c implementation instead.
I actually do use incremental hashing later on. BPF currently
vmallocs() a big temporary buffer just so it can fill it and hash it.
I change it to hash as it goes.
I painted the bike shed the other way because I thought that crypto_
names should indicate that they're the versions compatible with the
crypto API, but I take your point about churn. Part of the reason I
didn't want to use crypto_sha256_update is because that function is
currently like this:
int crypto_sha256_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
and I wanted to avoid churn. The sha256_update() functions scattered
all over were static, so I didn't worry about them.
I'm going to give this another try as a split-up series that tries to
avoid making any changes beyond simple function renames to the
drivers.
>
> Also, I strongly feel that crypto and other security related patches
> should be tested before being posted, even if they are only RFC,
> especially when they are posted by high profile kernel devs like
> yourself. (Your code incorrectly calls crypto_sha2_final() in a couple
> of places, resulting in the finalization being performed twice, once
> with the accelerated block transform and again with the generic
> transform)
>
I tested it, albeit poorly. I wanted feedback on the API (thanks!)
and I figured I could more carefully check the implementation once the
API survives a bit of review. Since it looks like I have to rework
this, I'd need to re-test anyway.
>> I suspect this will very slightly speed up the SHA256 shash operations
>> as well by reducing the amount of indirection involved.
>>
>
> I think you have a valid point when it comes to the complexity of the
> crypto API in general. But the struct sha256_state is embedded in the
> shash_desc rather than referred to via a pointer, so the level of
> indirection does not appear to change. And given how 99.9% of the
> SHA256 execution time is spent in the block transform routine anyway,
> I expect the performance delta to be in the noise tbh.
s/very slightly/negligibly? There's an extra speedup from avoiding a
variable-length stack allocation, but that probably doesn't matter
much either.
>
> Finally, another thing to keep in mind is that the base layers of
> SHA-1, SHA-256 and SHA-512 are intentionally structured in the same
> way. If there is a need for a digest() entry point, I'd prefer to add
> them for all flavours.
I want to get sha256 right first. Once it's in good shape, making the
same changes to the other variants should be easy.
>
> Whether this still belongs under crypto or under lib/sha256.c as a
> library function (allowing archs to override it) is open for debate.
> If hashing BPF programs becomes a hot spot, we probably have bigger
> problems.
>
> Regards,
> Ard.
>
> P.S. I do take your point regarding the arch_sha256_block_transform()
> proposed in your follow up email, but there are some details (SIMD,
> availability of the instructions etc) that would make it only suitable
> for the generic implementation anyway, and the base layer was already
> a huge improvement compared to the open coded implementations of the
> SHA boilerplate.
Agreed, and my model may not be quite right. It might have to be
something like:
if (arch_begin_sha256(len)) {
... do it with arch helpers...
arch_end_sha256();
} else {
... do it generically ...
}
>> - return sha256_base_finish(desc, out);
>> + return crypto_sha2_final(desc, out);
>
> This is wrong: your crypto_sha2_final also calls sha256_base_do_finalize()
Ugh, right. I clearly need to organize this change better to avoid
this kind of mistake.
^ permalink raw reply
* Re: [PATCH 3/6] wl1251: Update wl->nvs_len after wl->nvs is valid
From: Pavel Machek @ 2016-12-24 18:02 UTC (permalink / raw)
To: Pali Rohár
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-4-git-send-email-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]
On Sat 2016-12-24 17:52:58, Pali Rohár wrote:
> In case kmemdup fails thne wl->nvs_len will contains invalid non-zero size.
> This patch fixes it.
If kmemdup fails, then wl->nvs_len will contain invalid non-zero size.
?
This probably should go as first in series, as it is bugfix?
Pavel
Acked-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> drivers/net/wireless/ti/wl1251/main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
> index 24f8866..8971b64 100644
> --- a/drivers/net/wireless/ti/wl1251/main.c
> +++ b/drivers/net/wireless/ti/wl1251/main.c
> @@ -124,8 +124,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
> goto out;
> }
>
> - wl->nvs_len = fw->size;
> - wl->nvs = kmemdup(fw->data, wl->nvs_len, GFP_KERNEL);
> + wl->nvs = kmemdup(fw->data, fw->size, GFP_KERNEL);
>
> if (!wl->nvs) {
> wl1251_error("could not allocate memory for the nvs file");
> @@ -133,6 +132,8 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
> goto out;
> }
>
> + wl->nvs_len = fw->size;
> +
> ret = 0;
>
> out:
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid
From: Pavel Machek @ 2016-12-24 18:08 UTC (permalink / raw)
To: Pali Rohár
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-5-git-send-email-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 991 bytes --]
On Sat 2016-12-24 17:52:59, Pali Rohár wrote:
> Before this patch driver generated random MAC address every time when was
> doing initialization. And after that random MAC address could be
> overwritten with fixed one if provided.
Before this patch, driver generated random MAC address every time it
was initialized. After that random MAC address could be overwritten
with fixed one, if provided.
> This patch changes order. First it tries to read fixed MAC address and if
> it fails then driver generates random MAC address.
I don't quite get where the advantage is supposed to be. Is it that
"use_eeprom" is set, but reading fails?
The only case where this helps is if wl1251_read_eeprom_mac() succeeds
but reads invalid address.
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 5/6] wl1251: Parse and use MAC address from supplied NVS data
From: Pavel Machek @ 2016-12-24 18:14 UTC (permalink / raw)
To: Pali Rohár
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-6-git-send-email-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3501 bytes --]
On Sat 2016-12-24 17:53:00, Pali Rohár wrote:
> This patch implements parsing MAC address from NVS data which are sent to
> wl1251 chip. Calibration NVS data could contain valid MAC address and it
> will be used instead randomly generated.
will be used instead of randomly generated one.
> This patch also move code for requesting NVS data from userspace to driver
"moves"
> initialization code to make sure that NVS data will be there at time when
> permanent MAC address is needed.
"at a time"
> Calibration NVS data for wl1251 are model specific. Every one device with
"device specific"? "Every device".
> wl1251 chip should have been calibrated in factory and needs to provide own
> calibration data.
>
> Default example wl1251-nvs.bin data found in linux-firmware repository and
"are found"
> contains MAC address 00:00:20:07:03:09. So this MAC address is marked as
"contain"
> invalid as it is not real device specific address, just example one.
>
> Format of calibration NVS data can be found at:
> http://notaz.gp2x.de/misc/pnd/wl1251/nvs_map.txt
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> drivers/net/wireless/ti/wl1251/main.c | 39 ++++++++++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
> index c3fa0b6..1454ba2 100644
> --- a/drivers/net/wireless/ti/wl1251/main.c
> +++ b/drivers/net/wireless/ti/wl1251/main.c
> @@ -205,13 +205,6 @@ static int wl1251_chip_wakeup(struct wl1251 *wl)
> goto out;
> }
>
> - if (wl->nvs == NULL && !wl->use_eeprom) {
> - /* No NVS from netlink, try to get it from the filesystem */
> - ret = wl1251_fetch_nvs(wl);
> - if (ret < 0)
> - goto out;
> - }
> -
> out:
> return ret;
> }
> @@ -1538,6 +1531,30 @@ static int wl1251_read_eeprom_mac(struct wl1251 *wl)
> return 0;
> }
>
> +static int wl1251_read_nvs_mac(struct wl1251 *wl)
> +{
> + u8 mac[ETH_ALEN];
> + int i;
> +
> + if (wl->nvs_len < 0x24)
> + return -ENODATA;
> +
> + /* length is 2 and data address is 0x546c (mask is 0xfffe) */
> + if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b] != 0x54)
> + return -EINVAL;
> +
> + /* MAC is stored in reverse order */
> + for (i = 0; i < ETH_ALEN; i++)
> + mac[i] = wl->nvs[0x1c + ETH_ALEN - i - 1];
> +
> + /* 00:00:20:07:03:09 is in default example wl1251-nvs.bin, so invalid */
remove "default".
> + if (ether_addr_equal_unaligned(mac, "\x00\x00\x20\x07\x03\x09"))
> + return -EINVAL;
> +
> + memcpy(wl->mac_addr, mac, ETH_ALEN);
> + return 0;
> +}
> +
> static int wl1251_register_hw(struct wl1251 *wl)
> {
> int ret;
> @@ -1581,10 +1598,16 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
>
> wl->hw->queues = 4;
>
> + if (wl->nvs == NULL && !wl->use_eeprom) {
> + ret = wl1251_fetch_nvs(wl);
> + if (ret < 0)
> + goto out;
> + }
Is goto out here good idea? IMNSHO it is copy&paste bug, it should
just proceed with generating random address.
> if (wl->use_eeprom)
> ret = wl1251_read_eeprom_mac(wl);
> else
> - ret = -EINVAL;
> + ret = wl1251_read_nvs_mac(wl);
>
> if (ret == 0 && !is_valid_ether_addr(wl->mac_addr))
> ret = -EINVAL;
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data
From: Pavel Machek @ 2016-12-24 18:17 UTC (permalink / raw)
To: Pali Rohár
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-7-git-send-email-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 960 bytes --]
Hi!
> In case there is no valid MAC address kernel generates random one. This
> patch propagate this generated MAC address back to NVS data which will be
> uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
> uses.
> return 0;
> }
>
> +static int wl1251_write_nvs_mac(struct wl1251 *wl)
> +{
The name is quite confusing, this sounds like writing into
non-volatile storage.
> + int i;
> +
> + if (wl->nvs_len < 0x24)
> + return -ENODATA;
> +
> + /* length is 2 and data address is 0x546c (mask is 0xfffe) */
You don't actually check for the mask.
> + if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b] != 0x54)
> + return -EINVAL;
You have two copies of these. Does it make sense to move it to helper
function?
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid
From: Pali Rohár @ 2016-12-24 18:38 UTC (permalink / raw)
To: Pavel Machek
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <20161224180854.GB13590@amd>
[-- Attachment #1: Type: Text/Plain, Size: 1251 bytes --]
On Saturday 24 December 2016 19:08:54 Pavel Machek wrote:
> On Sat 2016-12-24 17:52:59, Pali Rohár wrote:
> > Before this patch driver generated random MAC address every time
> > when was doing initialization. And after that random MAC address
> > could be overwritten with fixed one if provided.
>
> Before this patch, driver generated random MAC address every time it
> was initialized. After that random MAC address could be overwritten
> with fixed one, if provided.
>
> > This patch changes order. First it tries to read fixed MAC address
> > and if it fails then driver generates random MAC address.
>
> I don't quite get where the advantage is supposed to be. Is it that
> "use_eeprom" is set, but reading fails?
Random bytes are read from kernel only if random MAC address is needed.
And in wl->mac_addr is always either invalid address or permanenent mac
address which will be used. Without patch in wl->mac_addr can be random
temporary address for some time...
> The only case where this helps is if wl1251_read_eeprom_mac()
> succeeds but reads invalid address.
>
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 5/6] wl1251: Parse and use MAC address from supplied NVS data
From: Pali Rohár @ 2016-12-24 18:40 UTC (permalink / raw)
To: Pavel Machek
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <20161224181421.GC13590@amd>
[-- Attachment #1: Type: Text/Plain, Size: 663 bytes --]
On Saturday 24 December 2016 19:14:21 Pavel Machek wrote:
> On Sat 2016-12-24 17:53:00, Pali Rohár wrote:
> > @@ -1581,10 +1598,16 @@ int wl1251_init_ieee80211(struct wl1251
> > *wl)
> >
> > wl->hw->queues = 4;
> >
> > + if (wl->nvs == NULL && !wl->use_eeprom) {
> > + ret = wl1251_fetch_nvs(wl);
> > + if (ret < 0)
> > + goto out;
> > + }
>
> Is goto out here good idea? IMNSHO it is copy&paste bug, it should
> just proceed with generating random address.
No, goto is correct here. wl1251 cannot be initialized without NVS data.
And when fetching (from userspace) fails it is fatal error.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data
From: Pali Rohár @ 2016-12-24 18:44 UTC (permalink / raw)
To: Pavel Machek
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <20161224181730.GD13590@amd>
[-- Attachment #1: Type: Text/Plain, Size: 1277 bytes --]
On Saturday 24 December 2016 19:17:30 Pavel Machek wrote:
> Hi!
>
> > In case there is no valid MAC address kernel generates random one.
> > This patch propagate this generated MAC address back to NVS data
> > which will be uploaded to wl1251 chip. So HW would have same MAC
> > address as linux kernel uses.
> >
> > return 0;
> >
> > }
> >
> > +static int wl1251_write_nvs_mac(struct wl1251 *wl)
> > +{
>
> The name is quite confusing, this sounds like writing into
> non-volatile storage.
>
> > + int i;
> > +
> > + if (wl->nvs_len < 0x24)
> > + return -ENODATA;
> > +
> > + /* length is 2 and data address is 0x546c (mask is 0xfffe) */
>
> You don't actually check for the mask.
It is quite complicated. { 0x6d, 0x54 } (= 0x546d) in data represent
address 0x546c and content are data. You need to apply mask 0xfffe for
0x546d and you get address where data will be written (so 0x546c).
> > + if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b]
> > != 0x54) + return -EINVAL;
>
> You have two copies of these. Does it make sense to move it to helper
> function?
I'm thinking if checks is really needed. But probably moving it to
separate function is good idea.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value
From: Sergei Shtylyov @ 2016-12-24 19:06 UTC (permalink / raw)
To: Thomas Preisner, dave
Cc: netdev, linux-kernel, linux-kernel, milan.stephan+linux
In-Reply-To: <1482580958-15406-2-git-send-email-thomas.preisner+linux@fau.de>
Hello!
On 12/24/2016 03:02 PM, Thomas Preisner wrote:
> In a few cases the err-variable is not set to a negative error code if a
> function call fails and thus 0 is returned instead.
> It may be better to set err to the appropriate negative error code
> before returning.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841
>
> Reported-by: Pan Bian <bianpan2016@163.com>
> Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
> Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
> ---
> drivers/net/ethernet/3com/typhoon.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
> index a0cacbe..c88b88a 100644
> --- a/drivers/net/ethernet/3com/typhoon.c
> +++ b/drivers/net/ethernet/3com/typhoon.c
[...]
> @@ -2411,7 +2412,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> * later when we print out the version reported.
> */
> INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
> - if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
> + err = typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp);
> + if(err < 0) {
Need a space between *if* and (. Run your patches thru
scripts/checkpatch.pl before posting, please.
> err_msg = "Could not get Sleep Image version";
> goto error_out_reset;
> }
> @@ -2453,7 +2455,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> dev->features = dev->hw_features |
> NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_RXCSUM;
>
> - if(register_netdev(dev) < 0) {
> + err = register_netdev(dev);
> + if(err < 0) {
Same here.
[...]
MBR, Sergei
^ permalink raw reply
* Re: [RFC PATCH 4.10 3/6] bpf: Use SHA256 instead of SHA1 for bpf digests
From: Daniel Borkmann @ 2016-12-24 19:59 UTC (permalink / raw)
To: Andy Lutomirski, Netdev, LKML, Linux Crypto Mailing List
Cc: Jason A. Donenfeld, Hannes Frederic Sowa, Alexei Starovoitov,
Eric Dumazet, Eric Biggers, Tom Herbert, David S. Miller,
Alexei Starovoitov
In-Reply-To: <bbfd69226fd74391045bafc695bba9a46cacca85.1482545792.git.luto@kernel.org>
On 12/24/2016 03:22 AM, Andy Lutomirski wrote:
> BPF digests are intended to be used to avoid reloading programs that
> are already loaded. For use cases (CRIU?) where untrusted programs
> are involved, intentional hash collisions could cause the wrong BPF
> program to execute. Additionally, if BPF digests are ever used
> in-kernel to skip verification, a hash collision could give privilege
> escalation directly.
Just for the record, digests will never ever be used to skip the
verification step, so I don't know why this idea even comes up
here (?) or is part of the changelog? As this will never be done
anyway, rather drop that part so we can avoid confusion on this?
Wrt untrusted programs, I don't see much of a use on this facility
in general for them. Something like a tail call map would quite
likely only be private to the application. And again, I really doubt
we'll have something like user namespace support in the foreseeable
future. Anyway, that said, I don't really have a big issue if you
want to switch to sha256, though.
> SHA1 is no longer considered adequately collision-resistant (see, for
> example, all the major browsers dropping support for SHA1
> certificates). Use SHA256 instead.
>
> I moved the digest field to keep all of the bpf program metadata in
> the same cache line.
>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
^ permalink raw reply
* Re: [PATCH net] net, sched: fix soft lockup in tc_classify
From: Daniel Borkmann @ 2016-12-24 21:03 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Shahar Klein, Or Gerlitz, Roi Dayan, Jiri Pirko,
John Fastabend, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpX2X-WHbf1VxfQzh_-YUEqk=o6B+uYfYhj_45jJGaFSfQ@mail.gmail.com>
On 12/24/2016 08:34 AM, Cong Wang wrote:
> On Thu, Dec 22, 2016 at 4:26 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 12/22/2016 08:05 PM, Cong Wang wrote:
>>> On Wed, Dec 21, 2016 at 1:07 PM, Daniel Borkmann <daniel@iogearbox.net>
>>> wrote:
>>>>
>>>> Ok, you mean for net. In that case I prefer the smaller sized fix to be
>>>> honest. It also covers everything from the point where we fetch the chain
>>>> via cops->tcf_chain() to the end of the function, which is where most of
>>>> the complexity resides, and only the two mentioned commits do the relock,
>>>
>>> I really wish the problem is only about relocking, but look at the code,
>>> the deeper reason why we have this bug is the complexity of the logic
>>> inside tc_ctl_tfilter(): 1) the replay logic is hard, we have to make it
>>> idempotent; 2) the request logic itself is hard, because of tc filter
>>> design
>>> and implementation.
>>>
>>> This is why I worry more than just relocking.
>>
>> But do you have a concrete 2nd issue/bug you're seeing? It rather sounds to
>> me your argument is more about fear of complexity on tc framework itself.
>> I agree it's complex, and tc_ctl_tfilter() is quite big in itself, where it
>> would be good to reduce it's complexity into smaller pieces. But it's not
>> really related to the fix itself, reducing complexity requires significantly
>> more and deeper work on the code. We can rework tc_ctl_tfilter() in net-next
>> to try to simplify it, sure, but I don't get why we have to discuss so much
>> on this matter in this context, really.
>
> Thanks for ignoring my point 1) above... You are dragging the discussion
> further.
I don't think so. The analysis and patch I proposed provides an explanation
of how we get into the seen endless loop, it provides a logical fix for it,
which has been reviewed by others and it has been tested extensively that it
resolves the issue, which was easily reproducible for the reporter and that
after the fix it never occurred again. The delta is absolutely simple and
really low risk. Given this function has not much changed over time, also
distros could pick it up that have a much older base kernel than current
stable ones. This initiated follow-up discussion we're having here in general
is dragging the focus away for everyone, and quite frankly I'm getting tired
of discussing it. I have stated my preferences, you have stated yours, and
we're only repeating ourselves in circles which isn't helpful in any way,
the discussion is not about some concrete bug in the logic to fix anymore
(otherwise please name it). Hence my proposal that everything else can wait
and be done in net-next.
^ permalink raw reply
* Re: [PATCHv2 1/5] sh_eth: add generic wake-on-lan support via magic packet
From: Sergei Shtylyov @ 2016-12-24 21:53 UTC (permalink / raw)
To: Geert Uytterhoeven, Niklas Söderlund
Cc: Simon Horman, netdev@vger.kernel.org, Linux-Renesas
In-Reply-To: <CAMuHMdXVVS_eLozvDnEz67_Fwg7jGDK0YEGh84rS+EAm4RFnag@mail.gmail.com>
Hello!
On 12/19/2016 08:11 PM, Geert Uytterhoeven wrote:
>>>> One quirk needed for WoL is that the module clock needs to be prevented
>>>> from being switched off by Runtime PM. To keep the clock alive the
>>>
>>> I tried to find the code in question and failed, getting muddled in the
>>> RPM maze. Could you point at this code for my education? :-)
>>
>> In my investigation I observed this (simplified) call graph with regards
>> to clocks for suspend:
>>
>> pm_suspend
There's a long list of the calls skipped here. :-)
>> pm_clk_suspend
>> clk_disable
>> clk_core_disable
>> cpg_mstp_clock_disable
>>
>> The interesting function here are clk_core_disable(). In that function a
>> 'enable_count' for each clock is decremented and the clock is only
>> turned of if the count reaches zero, hence cpg_mstp_clock_disable() are
>> only called if the counter reaches 0. At runtime the enable_count can be
>> displayed by examining /sys/kernel/debug/clk/clk_summary.
Well, this is not new to me... it's more interesting how we get there... :-)
[...]
>>>> usage count of the clock. Then when Runtime PM decreases the clock usage
>>>> count it won't reach 0 and be switched off.
>>>
>>> You mean it does this even though we don't call pr_runtime_put_sync()
>>> as done in sh_eth_close()?
>>
>> Yes.
>>
>> I had a look at the pm_runtime_* functions in include/linux/pm_runtime.h
>> and drivers/base/power/runtime.c and could not find any clock handling.
>> Maybe they only deal with power domains?
>
> There should be a generic way to prevent a device from being suspended.
Indeed.
> This will make sure the module clock is not disabled, and the power domain
> (if applicable) is not powered down.
I've just bumped into <linux/pm_wakeirq.h>, it looks promising...
[...]
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergei
^ permalink raw reply
* Re: Re: [PATCH v2 1/2] drivers: net: ethernet: 3com: fix return value
From: Thomas Preisner @ 2016-12-25 0:30 UTC (permalink / raw)
To: sergei.shtylyov
Cc: dave, netdev, linux-kernel, linux-kernel, milan.stephan+linux,
thomas.preisner+linux
In-Reply-To: <4200be74-f7e7-db6b-a258-8fd178fef369@cogentembedded.com>
Hello.
On Sat, 2016-12-24 at 20:06 +0100, Sergei Shtylyov wrote:
>Hello!
>
>On 12/24/2016 03:02 PM, Thomas Preisner wrote:
>
>> In a few cases the err-variable is not set to a negative error code if a
>> function call fails and thus 0 is returned instead.
>> It may be better to set err to the appropriate negative error code
>> before returning.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188841
>>
>> Reported-by: Pan Bian <bianpan2016@163.com>
>> Signed-off-by: Thomas Preisner <thomas.preisner+linux@fau.de>
>> Signed-off-by: Milan Stephan <milan.stephan+linux@fau.de>
>> ---
>> drivers/net/ethernet/3com/typhoon.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
>> index a0cacbe..c88b88a 100644
>> --- a/drivers/net/ethernet/3com/typhoon.c
>> +++ b/drivers/net/ethernet/3com/typhoon.c
>[...]
>> @@ -2411,7 +2412,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>> * later when we print out the version reported.
>> */
>> INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
>> - if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
>> + err = typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp);
>> + if(err < 0) {
>
> Need a space between *if* and (. Run your patches thru
>scripts/checkpatch.pl before posting, please.
Those spaces were actually left out purposely: The file in question (typhoon.c)
is missing those spaces between the statements (if, for, while) and the
following opening bracket pretty much always (except 2-3 times) and we figured
that it might be better to keep the coding style consistent since this might
aswell have been intended by the original author.
>
>> err_msg = "Could not get Sleep Image version";
>> goto error_out_reset;
>> }
>> @@ -2453,7 +2455,8 @@ typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>> dev->features = dev->hw_features |
>> NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_RXCSUM;
>>
>> - if(register_netdev(dev) < 0) {
>> + err = register_netdev(dev);
>> + if(err < 0) {
>
> Same here.
>
>[...]
>
>MBR, Sergei
But of course we can provide you with a patchset including those spaces.
With Regards,
Milan and Thomas
^ permalink raw reply
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