* Re: [PATCH v3 net-next 1/2] net: dsa: lan9303: Add port_fast_age and port_fdb_dump methods
From: Vivien Didelot @ 2017-10-20 13:57 UTC (permalink / raw)
To: Egil Hjelmeland, andrew, f.fainelli, netdev, linux-kernel; +Cc: Egil Hjelmeland
In-Reply-To: <20171020101910.2245-2-privat@egil-hjelmeland.no>
Hi Egil,
Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
> Add DSA method port_fast_age as a step to STP support.
>
> Add low level functions for accessing the lan9303 ALR (Address Logic
> Resolution).
>
> Added DSA method port_fdb_dump
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: dsa: lan9303: Add fdb/mdb manipulation
From: Vivien Didelot @ 2017-10-20 13:57 UTC (permalink / raw)
To: Egil Hjelmeland, andrew, f.fainelli, netdev, linux-kernel; +Cc: Egil Hjelmeland
In-Reply-To: <20171020101910.2245-3-privat@egil-hjelmeland.no>
Hi Egil,
Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
> Add functions for managing the lan9303 ALR (Address Logic
> Resolution).
>
> Implement DSA methods: port_fdb_add, port_fdb_del, port_mdb_prepare,
> port_mdb_add and port_mdb_del.
>
> Since the lan9303 do not offer reading specific ALR entry, the driver
> caches all static entries - in a flat table.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH v3 net-next 0/2] net: dsa: lan9303: Add fdb/mdb methods
From: Vivien Didelot @ 2017-10-20 14:03 UTC (permalink / raw)
To: Egil Hjelmeland, andrew, f.fainelli, netdev, linux-kernel; +Cc: Egil Hjelmeland
In-Reply-To: <20171020101910.2245-1-privat@egil-hjelmeland.no>
Hi Egil,
It looks much cleaner, thanks!
Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
> This series add support for accessing and managing the lan9303 ALR
> (Address Logic Resolution).
>
> The first patch add low level functions for accessing the ALR, along
> with port_fast_age and port_fdb_dump methods.
>
> The second patch add functions for managing ALR entires, along with
> remaining fdb/mdb methods.
>
> Note that to complete STP support, a special ALR entry with the STP eth
> address must be added too. This must be addressed later.
>
> Comments welcome!
>
>
> Changes v2 -> v3:
> - Whitespace polishing. Removed some "section" comments.
> - Prefixed ALR constants with LAN9303_ for consistency.
> - Patch 2: lan9303_port_fast_age() wrap the "port" into a struct for passing
> as context to alr_loop_cb_del_port_learned. Safer in event of type change.
> - Patch 2: Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
You'll usually directly add the Reviewed-by: tags right under your
Signed-off-by: tag of the concerned patches in next iterations.
(don't worry though I've added it back to patch 2/2.)
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Steve Lin @ 2017-10-20 14:03 UTC (permalink / raw)
To: Jiri Pirko
Cc: Yuval Mintz, netdev@vger.kernel.org, Jiri Pirko,
davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <20171019213955.GG1978@nanopsycho.orion>
On Thu, Oct 19, 2017 at 5:39 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuvalm@mellanox.com wrote:
>>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent
>>> config
>>> parameter. Defines number of MSI-X vectors allocated per VF.
>>> Value is permanent (stored in NVRAM), so becomes the new default
>>> value for this device.
>>
>>Sounds like you're having this enforce the same configuration for all child VFs.
>
> Yeah, this sounds like per-port config.
>
Well, it gets a little tricky here. I assume some cards handle this
per-port. Other cards might handle this per PF, where PF may not
always correspond 1:1 with a port. And some cards maybe just allow a
single value for this parameter for the entire card, covering all
ports/PFs.
To keep things simple and as general as possible, it made sense to set
all parameters on a per-PCI device level. As I mentioned in my
cover-letter, the devices most likely to use these proposed commands
do not have a single "whole asic" PCI b/d/f with internal mechanism
for accessing ports - most expose each port (and each function on each
port) as a separate PCI b/d/f, with no separate "whole asic" PCI
b/d/f. That's how the BCM cards work, and I think that's how the MLNX
cards work, and others that would be likely to use these cmds.
So, to summarize, you direct the command to the PCI b/d/f you want to
target. Does this make sense?
^ permalink raw reply
* Re: [PATCH v3 net-next 0/2] net: dsa: lan9303: Add fdb/mdb methods
From: Egil Hjelmeland @ 2017-10-20 14:09 UTC (permalink / raw)
To: Vivien Didelot, andrew, f.fainelli, netdev, linux-kernel
In-Reply-To: <87d15h29y1.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On 20. okt. 2017 16:03, Vivien Didelot wrote:
> Hi Egil,
>
> It looks much cleaner, thanks!
>
> Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
>
>> This series add support for accessing and managing the lan9303 ALR
>> (Address Logic Resolution).
>>
>> The first patch add low level functions for accessing the ALR, along
>> with port_fast_age and port_fdb_dump methods.
>>
>> The second patch add functions for managing ALR entires, along with
>> remaining fdb/mdb methods.
>>
>> Note that to complete STP support, a special ALR entry with the STP eth
>> address must be added too. This must be addressed later.
>>
>> Comments welcome!
>>
>>
>> Changes v2 -> v3:
>> - Whitespace polishing. Removed some "section" comments.
>> - Prefixed ALR constants with LAN9303_ for consistency.
>> - Patch 2: lan9303_port_fast_age() wrap the "port" into a struct for passing
>> as context to alr_loop_cb_del_port_learned. Safer in event of type change.
>> - Patch 2: Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>
> You'll usually directly add the Reviewed-by: tags right under your
> Signed-off-by: tag of the concerned patches in next iterations.
>
Oh, I thought I had done it, but now I see I forgot to rebuild the
patches after that last minute git-amend where I put in your
Reviewed-by:.
> (don't worry though I've added it back to patch 2/2.)
>
>
> Thanks,
>
> Vivien
>
Egil
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Jiri Pirko @ 2017-10-20 14:10 UTC (permalink / raw)
To: Steve Lin
Cc: Yuval Mintz, netdev@vger.kernel.org, Jiri Pirko,
davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <CA+Jmh7HCC-CgU16E8vSm541KnNUKZUgWfT0tPVx73cJxREC0Ag@mail.gmail.com>
Fri, Oct 20, 2017 at 04:03:55PM CEST, steven.lin1@broadcom.com wrote:
>On Thu, Oct 19, 2017 at 5:39 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuvalm@mellanox.com wrote:
>>>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent
>>>> config
>>>> parameter. Defines number of MSI-X vectors allocated per VF.
>>>> Value is permanent (stored in NVRAM), so becomes the new default
>>>> value for this device.
>>>
>>>Sounds like you're having this enforce the same configuration for all child VFs.
>>
>> Yeah, this sounds like per-port config.
>>
>
>Well, it gets a little tricky here. I assume some cards handle this
>per-port. Other cards might handle this per PF, where PF may not
>always correspond 1:1 with a port. And some cards maybe just allow a
>single value for this parameter for the entire card, covering all
>ports/PFs.
>
>To keep things simple and as general as possible, it made sense to set
>all parameters on a per-PCI device level. As I mentioned in my
>cover-letter, the devices most likely to use these proposed commands
>do not have a single "whole asic" PCI b/d/f with internal mechanism
>for accessing ports - most expose each port (and each function on each
>port) as a separate PCI b/d/f, with no separate "whole asic" PCI
>b/d/f. That's how the BCM cards work, and I think that's how the MLNX
>cards work, and others that would be likely to use these cmds.
>
>So, to summarize, you direct the command to the PCI b/d/f you want to
>target. Does this make sense?
So you plan to have 1 devlink instance for each vf? Not sure that
does sound right to me :/
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Steve Lin @ 2017-10-20 14:19 UTC (permalink / raw)
To: Jiri Pirko
Cc: Yuval Mintz, netdev@vger.kernel.org, Jiri Pirko,
davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <20171020141010.GB1994@nanopsycho.orion>
On Fri, Oct 20, 2017 at 10:10 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Oct 20, 2017 at 04:03:55PM CEST, steven.lin1@broadcom.com wrote:
>>On Thu, Oct 19, 2017 at 5:39 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuvalm@mellanox.com wrote:
>>>>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent
>>>>> config
>>>>> parameter. Defines number of MSI-X vectors allocated per VF.
>>>>> Value is permanent (stored in NVRAM), so becomes the new default
>>>>> value for this device.
>>>>
>>>>Sounds like you're having this enforce the same configuration for all child VFs.
>>>
>>> Yeah, this sounds like per-port config.
>>>
>>
>>Well, it gets a little tricky here. I assume some cards handle this
>>per-port. Other cards might handle this per PF, where PF may not
>>always correspond 1:1 with a port. And some cards maybe just allow a
>>single value for this parameter for the entire card, covering all
>>ports/PFs.
>>
>>To keep things simple and as general as possible, it made sense to set
>>all parameters on a per-PCI device level. As I mentioned in my
>>cover-letter, the devices most likely to use these proposed commands
>>do not have a single "whole asic" PCI b/d/f with internal mechanism
>>for accessing ports - most expose each port (and each function on each
>>port) as a separate PCI b/d/f, with no separate "whole asic" PCI
>>b/d/f. That's how the BCM cards work, and I think that's how the MLNX
>>cards work, and others that would be likely to use these cmds.
>>
>>So, to summarize, you direct the command to the PCI b/d/f you want to
>>target. Does this make sense?
>
> So you plan to have 1 devlink instance for each vf? Not sure that
> does sound right to me :/
>
For the commands proposed in this patchset, AFAIK they all apply on a
per-PF or broader, i.e. per-port or whole-card, granularity, since
they affect permanent config that applies at boot-up. So, no, the VFs
don't really come into play here.
^ permalink raw reply
* Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
From: Jiri Pirko @ 2017-10-20 14:39 UTC (permalink / raw)
To: Steve Lin; +Cc: netdev, jiri, davem, michael.chan, linville, gospo
In-Reply-To: <1508440630-25830-2-git-send-email-steven.lin1@broadcom.com>
Thu, Oct 19, 2017 at 09:17:05PM CEST, steven.lin1@broadcom.com wrote:
>Add support for permanent config parameter get/set commands. Used
>for parameters held in NVRAM, persistent device configuration.
>
>Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>Acked-by: Andy Gospodarek <gospo@broadcom.com>
>---
> include/net/devlink.h | 3 +
> include/uapi/linux/devlink.h | 11 ++
> net/core/devlink.c | 234 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 248 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index b9654e1..bd64623 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -270,6 +270,9 @@ struct devlink_ops {
> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8 *p_encap_mode);
> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
>+ int (*perm_config_get)(struct devlink *devlink, u32 param, u32 *value);
>+ int (*perm_config_set)(struct devlink *devlink, u32 param, u32 value,
Please use enum instead of "u32 param". Also, what would happen if the
value is >u32, like string for example? I believe we need to take it into
the consideration for the UAPI sake.
[...]
>+
>+static int devlink_nl_single_param_set(struct sk_buff *msg,
>+ struct devlink *devlink,
>+ u32 param, u32 value)
>+{
>+ u32 orig_value;
>+ u8 need_restart;
>+ int err;
>+ const struct devlink_ops *ops = devlink->ops;
>+ struct nlattr *cfgparam_attr;
Reverse christmas tree please (this applies to all functions)
>+
>+ /* First get current value of parameter */
>+ err = ops->perm_config_get(devlink, param, &orig_value);
I'm missing why this is needed.
>+ if (err)
>+ return err;
>+
>+ /* Now set parameter */
>+ err = ops->perm_config_set(devlink, param, value, &need_restart);
>+ if (err)
>+ return err;
>+
>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
>+ /* Update restart reqd - if any param needs restart, should be set */
>+ if (need_restart)
>+ err = nla_put_u8(msg,
>+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, 1);
>+
>+ /* Since set was successful, write attr back to msg with orig val */
>+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param);
>+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, orig_value);
Why to write it back?
>+
>+ nla_nest_end(msg, cfgparam_attr);
>+
>+ return 0;
>+}
>+
>+static int devlink_nl_cmd_perm_config_set_doit(struct sk_buff *skb,
>+ struct genl_info *info)
>+{
>+ struct devlink *devlink = info->user_ptr[0];
>+ struct sk_buff *msg;
>+ void *hdr;
>+ struct nlattr *attr;
>+ int rem;
>+ int err;
>+ u8 restart_reqd = 0;
>+ struct nlattr *cfgparam_attr;
>+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
>+ u32 param;
>+ u32 value;
>+
>+ if (!devlink->ops || !devlink->ops->perm_config_get ||
>+ !devlink->ops->perm_config_set)
>+ return -EOPNOTSUPP;
>+
>+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>+ if (!msg)
>+ return -ENOMEM;
>+
>+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
>+ &devlink_nl_family, 0, DEVLINK_CMD_PERM_CONFIG_SET);
>+ if (!hdr) {
>+ err = -EMSGSIZE;
>+ goto nla_msg_failure;
>+ }
>+
>+ err = devlink_nl_put_handle(msg, devlink);
>+ if (err)
>+ goto nla_put_failure;
>+
>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIGS);
>+
>+ nla_for_each_nested(attr, info->attrs[DEVLINK_ATTR_PERM_CONFIGS], rem) {
>+ err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
>+ devlink_nl_policy, NULL);
>+ if (err)
>+ goto nla_nest_failure;
>+
>+ if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER] ||
>+ !tb[DEVLINK_ATTR_PERM_CONFIG_VALUE])
>+ continue;
>+
>+ param = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
You should check it the "param" value is withing the enum boundary.
>+ value = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_VALUE]);
>+ err = devlink_nl_single_param_set(msg, devlink, param,
>+ value);
>+ if (err)
>+ goto nla_nest_failure;
Wouldn't it make sense to rollback to old values if any of the config
parameters set would fail?
>+ }
>+
>+ nla_nest_end(msg, cfgparam_attr);
>+
>+ if (restart_reqd) {
>+ err = nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED,
>+ restart_reqd);
>+ if (err)
>+ goto nla_put_failure;
>+ }
>+
>+ genlmsg_end(msg, hdr);
>+ return genlmsg_reply(msg, info);
>+
>+nla_nest_failure:
>+ nla_nest_cancel(msg, cfgparam_attr);
>+nla_put_failure:
>+ genlmsg_cancel(msg, hdr);
>+nla_msg_failure:
>+ return err;
>+}
>+
> int devlink_dpipe_match_put(struct sk_buff *skb,
> struct devlink_dpipe_match *match)
> {
>@@ -2291,6 +2509,8 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
> [DEVLINK_ATTR_ESWITCH_ENCAP_MODE] = { .type = NLA_U8 },
> [DEVLINK_ATTR_DPIPE_TABLE_NAME] = { .type = NLA_NUL_STRING },
> [DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED] = { .type = NLA_U8 },
>+ [DEVLINK_ATTR_PERM_CONFIG_PARAMETER] = { .type = NLA_U32 },
>+ [DEVLINK_ATTR_PERM_CONFIG_VALUE] = { .type = NLA_U32 },
> };
>
> static const struct genl_ops devlink_nl_ops[] = {
>@@ -2451,6 +2671,20 @@ static const struct genl_ops devlink_nl_ops[] = {
> .flags = GENL_ADMIN_PERM,
> .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
> },
>+ {
>+ .cmd = DEVLINK_CMD_PERM_CONFIG_GET,
>+ .doit = devlink_nl_cmd_perm_config_get_doit,
>+ .policy = devlink_nl_policy,
>+ .flags = GENL_ADMIN_PERM,
>+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>+ },
>+ {
>+ .cmd = DEVLINK_CMD_PERM_CONFIG_SET,
>+ .doit = devlink_nl_cmd_perm_config_set_doit,
>+ .policy = devlink_nl_policy,
>+ .flags = GENL_ADMIN_PERM,
>+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>+ },
> };
>
> static struct genl_family devlink_nl_family __ro_after_init = {
>--
>2.7.4
>
^ permalink raw reply
* Re: v4.14-rc3/arm64 DABT exception in atomic_inc() / __skb_clone()
From: Wei Wei @ 2017-10-20 14:40 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arm-kernel, linux-kernel, netdev, edumazet, davem, willemb,
syzkaller
In-Reply-To: <20171020111408.edj24tztxdptte5r@lakrids.cambridge.arm.com>
Sadly, the syzkaller characterized it as a non-reproducible bug and there were empty
repro files. But if manually executing in VM like this “./syz-execprog -executor=
./syz-executor -repeat=0 -procs=16 -cover=0 crash-log”, it crashed when executing exactly
program 1056 using log0 provided.
I failed to generate the C reproducer with syz-repro as it said “no target compiler”
in the final step. I would appreciate if you could give some hints.
Thanks,
Wei
> On 20 Oct 2017, at 7:14 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Oct 19, 2017 at 10:16:08PM -0400, Wei Wei wrote:
>> Hi all,
>
> Hi,
>
>> I have fuzzed v4.14-rc3 using syzkaller and found a bug similar to that one [1].
>> But the call trace isn’t the same. The atomic_inc() might handle a corrupted
>> skb_buff.
>>
>> The logs and config have been uploaded to my github repo [2].
>>
>> [1] https://lkml.org/lkml/2017/10/2/216
>> [2] https://github.com/dotweiba/skb_clone_atomic_inc_bug
>
> These do look very similar to what I was hitting; all appear to be
> misaligned atomics in the same path.
>
> I see that you have some empty repro files in [2]. If you have any
> reproducers, would you mind sharing them?
>
> If any of those are smaller or more reliable than the one I was able to
> generate [3], it might make it more obvious what's going on, and/or make
> it simpler to come up with a plain C reproducer.
>
> Thanks,
> Mark.
>
> [3] https://www.kernel.org/pub/linux/kernel/people/mark/bugs/20171002-skb_clone-misaligned-atomic/syzkaller.repro
^ permalink raw reply
* Re: v4.14-rc3/arm64 DABT exception in atomic_inc() / __skb_clone()
From: Mark Rutland @ 2017-10-20 15:11 UTC (permalink / raw)
To: Wei Wei
Cc: linux-arm-kernel, linux-kernel, netdev, edumazet, davem, willemb,
syzkaller
In-Reply-To: <960D71EC-C1E9-4898-ACBE-543FC09483FF@gmail.com>
On Fri, Oct 20, 2017 at 10:40:38AM -0400, Wei Wei wrote:
> Sadly, the syzkaller characterized it as a non-reproducible bug and there were empty
> repro files. But if manually executing in VM like this “./syz-execprog -executor=
> ./syz-executor -repeat=0 -procs=16 -cover=0 crash-log”, it crashed when executing exactly
> program 1056 using log0 provided.
>
> I failed to generate the C reproducer with syz-repro as it said “no target compiler”
> in the final step. I would appreciate if you could give some hints.
syz-repro should produce a smaller syzkaller log before it tries to
generate a C file.
I use:
$ syz-repro -config qemu.cfg logN
... and in most cases it will eventually print a smaller log to the
console.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
From: Steve Lin @ 2017-10-20 15:13 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Netdev List, Jiri Pirko, David S . Miller, Michael Chan,
John Linville, Andy Gospodarek
In-Reply-To: <20171020143948.GC1994@nanopsycho.orion>
On Fri, Oct 20, 2017 at 10:39 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Oct 19, 2017 at 09:17:05PM CEST, steven.lin1@broadcom.com wrote:
>>Add support for permanent config parameter get/set commands. Used
>>for parameters held in NVRAM, persistent device configuration.
>>
>>Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>>Acked-by: Andy Gospodarek <gospo@broadcom.com>
>>---
>> include/net/devlink.h | 3 +
>> include/uapi/linux/devlink.h | 11 ++
>> net/core/devlink.c | 234 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 248 insertions(+)
>>
>>diff --git a/include/net/devlink.h b/include/net/devlink.h
>>index b9654e1..bd64623 100644
>>--- a/include/net/devlink.h
>>+++ b/include/net/devlink.h
>>@@ -270,6 +270,9 @@ struct devlink_ops {
>> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
>> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8 *p_encap_mode);
>> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
>>+ int (*perm_config_get)(struct devlink *devlink, u32 param, u32 *value);
>>+ int (*perm_config_set)(struct devlink *devlink, u32 param, u32 value,
>
> Please use enum instead of "u32 param". Also, what would happen if the
> value is >u32, like string for example? I believe we need to take it into
> the consideration for the UAPI sake.
>
>
Using enum instead of u32 param: ok, will do in v3, thanks.
Value > u32: In the RFC and v1 versions of the patch, each parameter
was its own attribute, so could have its own type (u32, string,
whatever). In v2, trying to move to nested parameters w/ parameter
being an enum, as requested, it seems to mean that the parameter value
now must be defined as a specific type, so I went with u32.
If we need to support strings or other types > u32, then the
perm_config_value attribute will not be a fixed type, so can't be
policy checked. Or, I could go back to non-nested as in RFC/v1 case
and have each parameter with its own type.
> [...]
>
>
>>+
>>+static int devlink_nl_single_param_set(struct sk_buff *msg,
>>+ struct devlink *devlink,
>>+ u32 param, u32 value)
>>+{
>>+ u32 orig_value;
>>+ u8 need_restart;
>>+ int err;
>>+ const struct devlink_ops *ops = devlink->ops;
>>+ struct nlattr *cfgparam_attr;
>
> Reverse christmas tree please (this applies to all functions)
Will do in v3, thanks.
>
>
>>+
>>+ /* First get current value of parameter */
>>+ err = ops->perm_config_get(devlink, param, &orig_value);
>
> I'm missing why this is needed.
>
>
>>+ if (err)
>>+ return err;
>>+
>>+ /* Now set parameter */
>>+ err = ops->perm_config_set(devlink, param, value, &need_restart);
>>+ if (err)
>>+ return err;
>>+
>>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
>>+ /* Update restart reqd - if any param needs restart, should be set */
>>+ if (need_restart)
>>+ err = nla_put_u8(msg,
>>+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, 1);
>>+
>>+ /* Since set was successful, write attr back to msg with orig val */
>>+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param);
>>+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, orig_value);
>
> Why to write it back?
In a response to the "RFC" version of this patch, you wrote: "Also,
we need to expose to the user the original value (currently being
used) and the new one (to be used after driver re-instatiation)".
I understood that to mean that we need to return the current/original
value of the parameter (and the user knows the new value, since they
are setting it).
If I mis-interpreted that comment, then I'm happy to remove returning
the original value to the user; it wasn't in there originally.
>
>
>>+
>>+ nla_nest_end(msg, cfgparam_attr);
>>+
>>+ return 0;
>>+}
>>+
>>+static int devlink_nl_cmd_perm_config_set_doit(struct sk_buff *skb,
>>+ struct genl_info *info)
>>+{
>>+ struct devlink *devlink = info->user_ptr[0];
>>+ struct sk_buff *msg;
>>+ void *hdr;
>>+ struct nlattr *attr;
>>+ int rem;
>>+ int err;
>>+ u8 restart_reqd = 0;
>>+ struct nlattr *cfgparam_attr;
>>+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
>>+ u32 param;
>>+ u32 value;
>>+
>>+ if (!devlink->ops || !devlink->ops->perm_config_get ||
>>+ !devlink->ops->perm_config_set)
>>+ return -EOPNOTSUPP;
>>+
>>+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>>+ if (!msg)
>>+ return -ENOMEM;
>>+
>>+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
>>+ &devlink_nl_family, 0, DEVLINK_CMD_PERM_CONFIG_SET);
>>+ if (!hdr) {
>>+ err = -EMSGSIZE;
>>+ goto nla_msg_failure;
>>+ }
>>+
>>+ err = devlink_nl_put_handle(msg, devlink);
>>+ if (err)
>>+ goto nla_put_failure;
>>+
>>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIGS);
>>+
>>+ nla_for_each_nested(attr, info->attrs[DEVLINK_ATTR_PERM_CONFIGS], rem) {
>>+ err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
>>+ devlink_nl_policy, NULL);
>>+ if (err)
>>+ goto nla_nest_failure;
>>+
>>+ if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER] ||
>>+ !tb[DEVLINK_ATTR_PERM_CONFIG_VALUE])
>>+ continue;
>>+
>>+ param = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
>
> You should check it the "param" value is withing the enum boundary.
Will do in v3, I'll add a DEVLINK_PERM_CONFIG_MAX enum and check against that.
>
>
>>+ value = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_VALUE]);
>>+ err = devlink_nl_single_param_set(msg, devlink, param,
>>+ value);
>>+ if (err)
>>+ goto nla_nest_failure;
>
> Wouldn't it make sense to rollback to old values if any of the config
> parameters set would fail?
Hmmmm.... We could do a rollback on failure of any of the set cmds,
but I feel like that's more useful when the user doesn't know which
sets failed. (i.e. if the response was just a boolean ok/not-ok
response).
If the user knows which sets worked and which didn't, then maybe no
rollback is necessary. So perhaps I change the code so that rather
than falling out on error, it continues to try all the set cmds, but
just returns the parameters which were successful.
Is there a convention for this? I'm fine either way.
>
>
>>+ }
>>+
>>+ nla_nest_end(msg, cfgparam_attr);
>>+
>>+ if (restart_reqd) {
>>+ err = nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED,
>>+ restart_reqd);
>>+ if (err)
>>+ goto nla_put_failure;
>>+ }
>>+
>>+ genlmsg_end(msg, hdr);
>>+ return genlmsg_reply(msg, info);
>>+
>>+nla_nest_failure:
>>+ nla_nest_cancel(msg, cfgparam_attr);
>>+nla_put_failure:
>>+ genlmsg_cancel(msg, hdr);
>>+nla_msg_failure:
>>+ return err;
>>+}
>>+
>> int devlink_dpipe_match_put(struct sk_buff *skb,
>> struct devlink_dpipe_match *match)
>> {
>>@@ -2291,6 +2509,8 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
>> [DEVLINK_ATTR_ESWITCH_ENCAP_MODE] = { .type = NLA_U8 },
>> [DEVLINK_ATTR_DPIPE_TABLE_NAME] = { .type = NLA_NUL_STRING },
>> [DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED] = { .type = NLA_U8 },
>>+ [DEVLINK_ATTR_PERM_CONFIG_PARAMETER] = { .type = NLA_U32 },
>>+ [DEVLINK_ATTR_PERM_CONFIG_VALUE] = { .type = NLA_U32 },
>> };
>>
>> static const struct genl_ops devlink_nl_ops[] = {
>>@@ -2451,6 +2671,20 @@ static const struct genl_ops devlink_nl_ops[] = {
>> .flags = GENL_ADMIN_PERM,
>> .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>> },
>>+ {
>>+ .cmd = DEVLINK_CMD_PERM_CONFIG_GET,
>>+ .doit = devlink_nl_cmd_perm_config_get_doit,
>>+ .policy = devlink_nl_policy,
>>+ .flags = GENL_ADMIN_PERM,
>>+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>>+ },
>>+ {
>>+ .cmd = DEVLINK_CMD_PERM_CONFIG_SET,
>>+ .doit = devlink_nl_cmd_perm_config_set_doit,
>>+ .policy = devlink_nl_policy,
>>+ .flags = GENL_ADMIN_PERM,
>>+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>>+ },
>> };
>>
>> static struct genl_family devlink_nl_family __ro_after_init = {
>>--
>>2.7.4
>>
^ permalink raw reply
* Re: v4.14-rc3/arm64 DABT exception in atomic_inc() / __skb_clone()
From: Dmitry Vyukov @ 2017-10-20 15:14 UTC (permalink / raw)
To: Wei Wei
Cc: Mark Rutland, linux-arm-kernel, LKML, netdev, Eric Dumazet,
David Miller, Willem de Bruijn, syzkaller
In-Reply-To: <960D71EC-C1E9-4898-ACBE-543FC09483FF@gmail.com>
On Fri, Oct 20, 2017 at 4:40 PM, Wei Wei <dotweiba@gmail.com> wrote:
> Sadly, the syzkaller characterized it as a non-reproducible bug and there were empty
> repro files. But if manually executing in VM like this “./syz-execprog -executor=
> ./syz-executor -repeat=0 -procs=16 -cover=0 crash-log”, it crashed when executing exactly
> program 1056 using log0 provided.
>
> I failed to generate the C reproducer with syz-repro as it said “no target compiler”
> in the final step. I would appreciate if you could give some hints.
syzkaller tries to use aarch64-linux-gnu-gcc when cross-compiling to arm64:
https://github.com/google/syzkaller/blob/master/sys/targets/targets.go#L62
Try to install g++-aarch64-linux-gnu.
Or how should it be done on your system?
> Thanks,
> Wei
>> On 20 Oct 2017, at 7:14 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>>
>> On Thu, Oct 19, 2017 at 10:16:08PM -0400, Wei Wei wrote:
>>> Hi all,
>>
>> Hi,
>>
>>> I have fuzzed v4.14-rc3 using syzkaller and found a bug similar to that one [1].
>>> But the call trace isn’t the same. The atomic_inc() might handle a corrupted
>>> skb_buff.
>>>
>>> The logs and config have been uploaded to my github repo [2].
>>>
>>> [1] https://lkml.org/lkml/2017/10/2/216
>>> [2] https://github.com/dotweiba/skb_clone_atomic_inc_bug
>>
>> These do look very similar to what I was hitting; all appear to be
>> misaligned atomics in the same path.
>>
>> I see that you have some empty repro files in [2]. If you have any
>> reproducers, would you mind sharing them?
>>
>> If any of those are smaller or more reliable than the one I was able to
>> generate [3], it might make it more obvious what's going on, and/or make
>> it simpler to come up with a plain C reproducer.
>>
>> Thanks,
>> Mark.
>>
>> [3] https://www.kernel.org/pub/linux/kernel/people/mark/bugs/20171002-skb_clone-misaligned-atomic/syzkaller.repro
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: v4.14-rc3/arm64 DABT exception in atomic_inc() / __skb_clone()
From: Willem de Bruijn @ 2017-10-20 15:39 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Wei Wei, Mark Rutland, linux-arm-kernel, LKML, netdev,
Eric Dumazet, David Miller, Willem de Bruijn, syzkaller
In-Reply-To: <CACT4Y+bjzzCv3LSxK12C1xB8GfR3UYCrX_5phRYJ8fpxh9vcvQ@mail.gmail.com>
On Fri, Oct 20, 2017 at 11:14 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Fri, Oct 20, 2017 at 4:40 PM, Wei Wei <dotweiba@gmail.com> wrote:
>> Sadly, the syzkaller characterized it as a non-reproducible bug and there were empty
>> repro files. But if manually executing in VM like this “./syz-execprog -executor=
>> ./syz-executor -repeat=0 -procs=16 -cover=0 crash-log”, it crashed when executing exactly
>> program 1056 using log0 provided.
>>
>> I failed to generate the C reproducer with syz-repro as it said “no target compiler”
>> in the final step. I would appreciate if you could give some hints.
>
> syzkaller tries to use aarch64-linux-gnu-gcc when cross-compiling to arm64:
> https://github.com/google/syzkaller/blob/master/sys/targets/targets.go#L62
> Try to install g++-aarch64-linux-gnu.
> Or how should it be done on your system?
A core dump would also be helpful to root around in and inspect
what those registers point to. Thanks for posting the various reports
on github, btw.
^ permalink raw reply
* Re: [PATCH net-next 6/8] tools: bpftool: print all relevant byte opcodes for "load double word"
From: Quentin Monnet @ 2017-10-20 15:50 UTC (permalink / raw)
To: David Laight
Cc: Jakub Kicinski, netdev@vger.kernel.org, oss-drivers@netronome.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD009D515@AcuExch.aculab.com>
Hi David,
On 20 October 2017 at 10:59, David Laight <David.Laight@aculab.com> wrote:
> From: Jakub Kicinski
>> Sent: 19 October 2017 23:46
>> The eBPF instruction permitting to load double words (8 bytes) into a
>> register need 8-byte long "immediate" field, and thus occupy twice the
>> space of other instructions. bpftool was aware of this and would
>> increment the instruction counter only once on meeting such instruction,
>> but it would only print the first four bytes of the immediate value to
>> load. Make it able to dump the whole 16 byte-long double instruction
>> instead (as would `llvm-objdump -d <program>`).
>
> Guess why most modern instruction sets use a 'load high' instruction
> to generate big constants...
>
> Interestingly, is there anything special in the rest of the
> second instruction in order to make it an identifiable no-op?
The remaining four bytes are taken from the "immediate" field of the second
instruction, which leaves the first four fields (offset, source and destination
registers, and in particular opcode) unused. As far as I know, these fields
remain at zero, and this makes it the only “instruction” to have a null code
(although I am not sure this is a strict requirement, because I did not find
the code in the verifier that would reject a program having a non-null opcode
right after a "load double word immediate" instruction).
>
[…]
> Why not just:
> for (i = 0; i < len / sizeof(*insn); i += 1 + double_insn) {
>
> ...
>
> David
>
Yes, we could use that instead, although I am not sure this makes the code
more readable. So I do not believe this is worth a respin, tell me if you think
otherwise.
Thanks for the review!
Quentin
^ permalink raw reply
* Re: [PATCH 18/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: jlee @ 2017-10-20 15:57 UTC (permalink / raw)
To: David Howells
Cc: Alexei Starovoitov, linux-security-module, gnomes, linux-efi,
matthew.garrett, gregkh, linux-kernel, jforbes, Daniel Borkmann,
David S. Miller, netdev, Gary Lin
In-Reply-To: <2582.1508486928@warthog.procyon.org.uk>
On Fri, Oct 20, 2017 at 09:08:48AM +0100, David Howells wrote:
> Hi Joey,
>
> Should I just lock down sys_bpf() entirely for now? We can always free it up
> somewhat later.
>
> David
OK~~ Please just remove my patch until we find out a way to
verify bpf code or protect sensitive data in memory.
I think that we don't need to lock down sys_bpf() now because
we didn't lock down other interfaces for reading arbitrary
address like /dev/mem and /dev/kmem.
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [PATCH v3 net] dccp/tcp: fix ireq->opt races
From: Eric Dumazet @ 2017-10-20 15:58 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20171020.071831.180922551241959965.davem@davemloft.net>
On Fri, 2017-10-20 at 07:18 +0100, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Fri, 20 Oct 2017 07:04:58 +0100 (WEST)
>
> > Much better, applied and queued up for -stable.
>
> I take that back.
>
> Please build test your changes more thoroughly.
Arg, sorry for wasting your time, I will send a v4.
^ permalink raw reply
* [PATCH net] rxrpc: Don't release call mutex on error pointer
From: David Howells @ 2017-10-20 16:01 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
Don't release call mutex at the end of rxrpc_kernel_begin_call() if the
call pointer actually holds an error value.
Fixes: 540b1c48c37a ("rxrpc: Fix deadlock between call creation and sendmsg/recvmsg")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/af_rxrpc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 73c980e26581..054e32872808 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -311,10 +311,11 @@ struct rxrpc_call *rxrpc_kernel_begin_call(struct socket *sock,
call = rxrpc_new_client_call(rx, &cp, srx, user_call_ID, tx_total_len,
gfp);
/* The socket has been unlocked. */
- if (!IS_ERR(call))
+ if (!IS_ERR(call)) {
call->notify_rx = notify_rx;
+ mutex_unlock(&call->user_mutex);
+ }
- mutex_unlock(&call->user_mutex);
_leave(" = %p", call);
return call;
}
^ permalink raw reply related
* Re: [PATCH 18/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: David Howells @ 2017-10-20 16:03 UTC (permalink / raw)
To: jlee-IBi9RG/b67k
Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA, Alexei Starovoitov,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jforbes-H+wXaHxf7aLQT0dZR+AlfA, Daniel Borkmann, David S. Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, Gary Lin
In-Reply-To: <20171020155748.kzrvg6565oxh6gmb@linux-rasp2>
jlee-IBi9RG/b67k@public.gmane.org wrote:
> I think that we don't need to lock down sys_bpf() now because
> we didn't lock down other interfaces for reading arbitrary
> address like /dev/mem and /dev/kmem.
Ummm... See patch 4. You even gave me a Reviewed-by for it ;-)
David
^ permalink raw reply
* [PATCH v4 net] tcp/dccp: fix ireq->opt races
From: Eric Dumazet @ 2017-10-20 16:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <1508451889.30291.4.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
syzkaller found another bug in DCCP/TCP stacks [1]
For the reasons explained in commit ce1050089c96 ("tcp/dccp: fix
ireq->pktopts race"), we need to make sure we do not access
ireq->opt unless we own the request sock.
Note the opt field is renamed to ireq_opt to ease grep games.
[1]
BUG: KASAN: use-after-free in ip_queue_xmit+0x1687/0x18e0 net/ipv4/ip_output.c:474
Read of size 1 at addr ffff8801c951039c by task syz-executor5/3295
CPU: 1 PID: 3295 Comm: syz-executor5 Not tainted 4.14.0-rc4+ #80
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:52
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:427
ip_queue_xmit+0x1687/0x18e0 net/ipv4/ip_output.c:474
tcp_transmit_skb+0x1ab7/0x3840 net/ipv4/tcp_output.c:1135
tcp_send_ack.part.37+0x3bb/0x650 net/ipv4/tcp_output.c:3587
tcp_send_ack+0x49/0x60 net/ipv4/tcp_output.c:3557
__tcp_ack_snd_check+0x2c6/0x4b0 net/ipv4/tcp_input.c:5072
tcp_ack_snd_check net/ipv4/tcp_input.c:5085 [inline]
tcp_rcv_state_process+0x2eff/0x4850 net/ipv4/tcp_input.c:6071
tcp_child_process+0x342/0x990 net/ipv4/tcp_minisocks.c:816
tcp_v4_rcv+0x1827/0x2f80 net/ipv4/tcp_ipv4.c:1682
ip_local_deliver_finish+0x2e2/0xba0 net/ipv4/ip_input.c:216
NF_HOOK include/linux/netfilter.h:249 [inline]
ip_local_deliver+0x1ce/0x6e0 net/ipv4/ip_input.c:257
dst_input include/net/dst.h:464 [inline]
ip_rcv_finish+0x887/0x19a0 net/ipv4/ip_input.c:397
NF_HOOK include/linux/netfilter.h:249 [inline]
ip_rcv+0xc3f/0x1820 net/ipv4/ip_input.c:493
__netif_receive_skb_core+0x1a3e/0x34b0 net/core/dev.c:4476
__netif_receive_skb+0x2c/0x1b0 net/core/dev.c:4514
netif_receive_skb_internal+0x10b/0x670 net/core/dev.c:4587
netif_receive_skb+0xae/0x390 net/core/dev.c:4611
tun_rx_batched.isra.50+0x5ed/0x860 drivers/net/tun.c:1372
tun_get_user+0x249c/0x36d0 drivers/net/tun.c:1766
tun_chr_write_iter+0xbf/0x160 drivers/net/tun.c:1792
call_write_iter include/linux/fs.h:1770 [inline]
new_sync_write fs/read_write.c:468 [inline]
__vfs_write+0x68a/0x970 fs/read_write.c:481
vfs_write+0x18f/0x510 fs/read_write.c:543
SYSC_write fs/read_write.c:588 [inline]
SyS_write+0xef/0x220 fs/read_write.c:580
entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x40c341
RSP: 002b:00007f469523ec10 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000718000 RCX: 000000000040c341
RDX: 0000000000000037 RSI: 0000000020004000 RDI: 0000000000000015
RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
R10: 00000000000f4240 R11: 0000000000000293 R12: 00000000004b7fd1
R13: 00000000ffffffff R14: 0000000020000000 R15: 0000000000025000
Allocated by task 3295:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
__do_kmalloc mm/slab.c:3725 [inline]
__kmalloc+0x162/0x760 mm/slab.c:3734
kmalloc include/linux/slab.h:498 [inline]
tcp_v4_save_options include/net/tcp.h:1962 [inline]
tcp_v4_init_req+0x2d3/0x3e0 net/ipv4/tcp_ipv4.c:1271
tcp_conn_request+0xf6d/0x3410 net/ipv4/tcp_input.c:6283
tcp_v4_conn_request+0x157/0x210 net/ipv4/tcp_ipv4.c:1313
tcp_rcv_state_process+0x8ea/0x4850 net/ipv4/tcp_input.c:5857
tcp_v4_do_rcv+0x55c/0x7d0 net/ipv4/tcp_ipv4.c:1482
tcp_v4_rcv+0x2d10/0x2f80 net/ipv4/tcp_ipv4.c:1711
ip_local_deliver_finish+0x2e2/0xba0 net/ipv4/ip_input.c:216
NF_HOOK include/linux/netfilter.h:249 [inline]
ip_local_deliver+0x1ce/0x6e0 net/ipv4/ip_input.c:257
dst_input include/net/dst.h:464 [inline]
ip_rcv_finish+0x887/0x19a0 net/ipv4/ip_input.c:397
NF_HOOK include/linux/netfilter.h:249 [inline]
ip_rcv+0xc3f/0x1820 net/ipv4/ip_input.c:493
__netif_receive_skb_core+0x1a3e/0x34b0 net/core/dev.c:4476
__netif_receive_skb+0x2c/0x1b0 net/core/dev.c:4514
netif_receive_skb_internal+0x10b/0x670 net/core/dev.c:4587
netif_receive_skb+0xae/0x390 net/core/dev.c:4611
tun_rx_batched.isra.50+0x5ed/0x860 drivers/net/tun.c:1372
tun_get_user+0x249c/0x36d0 drivers/net/tun.c:1766
tun_chr_write_iter+0xbf/0x160 drivers/net/tun.c:1792
call_write_iter include/linux/fs.h:1770 [inline]
new_sync_write fs/read_write.c:468 [inline]
__vfs_write+0x68a/0x970 fs/read_write.c:481
vfs_write+0x18f/0x510 fs/read_write.c:543
SYSC_write fs/read_write.c:588 [inline]
SyS_write+0xef/0x220 fs/read_write.c:580
entry_SYSCALL_64_fastpath+0x1f/0xbe
Freed by task 3306:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
__cache_free mm/slab.c:3503 [inline]
kfree+0xca/0x250 mm/slab.c:3820
inet_sock_destruct+0x59d/0x950 net/ipv4/af_inet.c:157
__sk_destruct+0xfd/0x910 net/core/sock.c:1560
sk_destruct+0x47/0x80 net/core/sock.c:1595
__sk_free+0x57/0x230 net/core/sock.c:1603
sk_free+0x2a/0x40 net/core/sock.c:1614
sock_put include/net/sock.h:1652 [inline]
inet_csk_complete_hashdance+0xd5/0xf0 net/ipv4/inet_connection_sock.c:959
tcp_check_req+0xf4d/0x1620 net/ipv4/tcp_minisocks.c:765
tcp_v4_rcv+0x17f6/0x2f80 net/ipv4/tcp_ipv4.c:1675
ip_local_deliver_finish+0x2e2/0xba0 net/ipv4/ip_input.c:216
NF_HOOK include/linux/netfilter.h:249 [inline]
ip_local_deliver+0x1ce/0x6e0 net/ipv4/ip_input.c:257
dst_input include/net/dst.h:464 [inline]
ip_rcv_finish+0x887/0x19a0 net/ipv4/ip_input.c:397
NF_HOOK include/linux/netfilter.h:249 [inline]
ip_rcv+0xc3f/0x1820 net/ipv4/ip_input.c:493
__netif_receive_skb_core+0x1a3e/0x34b0 net/core/dev.c:4476
__netif_receive_skb+0x2c/0x1b0 net/core/dev.c:4514
netif_receive_skb_internal+0x10b/0x670 net/core/dev.c:4587
netif_receive_skb+0xae/0x390 net/core/dev.c:4611
tun_rx_batched.isra.50+0x5ed/0x860 drivers/net/tun.c:1372
tun_get_user+0x249c/0x36d0 drivers/net/tun.c:1766
tun_chr_write_iter+0xbf/0x160 drivers/net/tun.c:1792
call_write_iter include/linux/fs.h:1770 [inline]
new_sync_write fs/read_write.c:468 [inline]
__vfs_write+0x68a/0x970 fs/read_write.c:481
vfs_write+0x18f/0x510 fs/read_write.c:543
SYSC_write fs/read_write.c:588 [inline]
SyS_write+0xef/0x220 fs/read_write.c:580
entry_SYSCALL_64_fastpath+0x1f/0xbe
Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
Fixes: 079096f103fa ("tcp/dccp: install syn_recv requests into ehash table")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
v4: Added the needed changes in net/ipv4/cipso_ipv4.c (and full build)
v3: fixed the white space mangling
v2: removed some lines from KASAN report that confuse patchwork.
include/net/inet_sock.h | 2 +-
net/dccp/ipv4.c | 13 ++++++++-----
net/ipv4/cipso_ipv4.c | 24 +++++++-----------------
net/ipv4/inet_connection_sock.c | 8 +++-----
net/ipv4/syncookies.c | 2 +-
net/ipv4/tcp_input.c | 2 +-
net/ipv4/tcp_ipv4.c | 22 +++++++++++++---------
7 files changed, 34 insertions(+), 39 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index aa95053dfc78d35d04aef276e2a5dce7343f72a0..425752f768d2f1a0efb13964204e07f27609e9db 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -96,7 +96,7 @@ struct inet_request_sock {
kmemcheck_bitfield_end(flags);
u32 ir_mark;
union {
- struct ip_options_rcu *opt;
+ struct ip_options_rcu __rcu *ireq_opt;
#if IS_ENABLED(CONFIG_IPV6)
struct {
struct ipv6_txoptions *ipv6_opt;
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 001c08696334bba0ceb896c116e595b814af0667..0490916864f93d5466e87f5b97dc524b3ee57a2e 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -414,8 +414,7 @@ struct sock *dccp_v4_request_recv_sock(const struct sock *sk,
sk_daddr_set(newsk, ireq->ir_rmt_addr);
sk_rcv_saddr_set(newsk, ireq->ir_loc_addr);
newinet->inet_saddr = ireq->ir_loc_addr;
- newinet->inet_opt = ireq->opt;
- ireq->opt = NULL;
+ RCU_INIT_POINTER(newinet->inet_opt, rcu_dereference(ireq->ireq_opt));
newinet->mc_index = inet_iif(skb);
newinet->mc_ttl = ip_hdr(skb)->ttl;
newinet->inet_id = jiffies;
@@ -430,7 +429,10 @@ struct sock *dccp_v4_request_recv_sock(const struct sock *sk,
if (__inet_inherit_port(sk, newsk) < 0)
goto put_and_exit;
*own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash));
-
+ if (*own_req)
+ ireq->ireq_opt = NULL;
+ else
+ newinet->inet_opt = NULL;
return newsk;
exit_overflow:
@@ -441,6 +443,7 @@ struct sock *dccp_v4_request_recv_sock(const struct sock *sk,
__NET_INC_STATS(sock_net(sk), LINUX_MIB_LISTENDROPS);
return NULL;
put_and_exit:
+ newinet->inet_opt = NULL;
inet_csk_prepare_forced_close(newsk);
dccp_done(newsk);
goto exit;
@@ -492,7 +495,7 @@ static int dccp_v4_send_response(const struct sock *sk, struct request_sock *req
ireq->ir_rmt_addr);
err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
ireq->ir_rmt_addr,
- ireq->opt);
+ rcu_dereference(ireq->ireq_opt));
err = net_xmit_eval(err);
}
@@ -548,7 +551,7 @@ static void dccp_v4_ctl_send_reset(const struct sock *sk, struct sk_buff *rxskb)
static void dccp_v4_reqsk_destructor(struct request_sock *req)
{
dccp_feat_list_purge(&dccp_rsk(req)->dreq_featneg);
- kfree(inet_rsk(req)->opt);
+ kfree(rcu_dereference_protected(inet_rsk(req)->ireq_opt, 1));
}
void dccp_syn_ack_timeout(const struct request_sock *req)
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 2ae8f54cb32148f2499f78ecbf29259db36bd207..82178cc69c9618bae69c096290a7a96a8b8bade0 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1951,7 +1951,7 @@ int cipso_v4_req_setattr(struct request_sock *req,
buf = NULL;
req_inet = inet_rsk(req);
- opt = xchg(&req_inet->opt, opt);
+ opt = xchg((__force struct ip_options_rcu **)&req_inet->ireq_opt, opt);
if (opt)
kfree_rcu(opt, rcu);
@@ -1973,11 +1973,13 @@ int cipso_v4_req_setattr(struct request_sock *req,
* values on failure.
*
*/
-static int cipso_v4_delopt(struct ip_options_rcu **opt_ptr)
+static int cipso_v4_delopt(struct ip_options_rcu __rcu **opt_ptr)
{
+ struct ip_options_rcu *opt = rcu_dereference_protected(*opt_ptr, 1);
int hdr_delta = 0;
- struct ip_options_rcu *opt = *opt_ptr;
+ if (!opt || opt->opt.cipso == 0)
+ return 0;
if (opt->opt.srr || opt->opt.rr || opt->opt.ts || opt->opt.router_alert) {
u8 cipso_len;
u8 cipso_off;
@@ -2039,14 +2041,10 @@ static int cipso_v4_delopt(struct ip_options_rcu **opt_ptr)
*/
void cipso_v4_sock_delattr(struct sock *sk)
{
- int hdr_delta;
- struct ip_options_rcu *opt;
struct inet_sock *sk_inet;
+ int hdr_delta;
sk_inet = inet_sk(sk);
- opt = rcu_dereference_protected(sk_inet->inet_opt, 1);
- if (!opt || opt->opt.cipso == 0)
- return;
hdr_delta = cipso_v4_delopt(&sk_inet->inet_opt);
if (sk_inet->is_icsk && hdr_delta > 0) {
@@ -2066,15 +2064,7 @@ void cipso_v4_sock_delattr(struct sock *sk)
*/
void cipso_v4_req_delattr(struct request_sock *req)
{
- struct ip_options_rcu *opt;
- struct inet_request_sock *req_inet;
-
- req_inet = inet_rsk(req);
- opt = req_inet->opt;
- if (!opt || opt->opt.cipso == 0)
- return;
-
- cipso_v4_delopt(&req_inet->opt);
+ cipso_v4_delopt(&inet_rsk(req)->ireq_opt);
}
/**
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 67aec7a106860b26c929fea1624d652c87972f04..5ec9136a7c36933cb36e5cd50058eb6cf189a7c3 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -540,9 +540,10 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
{
const struct inet_request_sock *ireq = inet_rsk(req);
struct net *net = read_pnet(&ireq->ireq_net);
- struct ip_options_rcu *opt = ireq->opt;
+ struct ip_options_rcu *opt;
struct rtable *rt;
+ opt = rcu_dereference(ireq->ireq_opt);
flowi4_init_output(fl4, ireq->ir_iif, ireq->ir_mark,
RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
sk->sk_protocol, inet_sk_flowi_flags(sk),
@@ -576,10 +577,9 @@ struct dst_entry *inet_csk_route_child_sock(const struct sock *sk,
struct flowi4 *fl4;
struct rtable *rt;
+ opt = rcu_dereference(ireq->ireq_opt);
fl4 = &newinet->cork.fl.u.ip4;
- rcu_read_lock();
- opt = rcu_dereference(newinet->inet_opt);
flowi4_init_output(fl4, ireq->ir_iif, ireq->ir_mark,
RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
sk->sk_protocol, inet_sk_flowi_flags(sk),
@@ -592,13 +592,11 @@ struct dst_entry *inet_csk_route_child_sock(const struct sock *sk,
goto no_route;
if (opt && opt->opt.is_strictroute && rt->rt_uses_gateway)
goto route_err;
- rcu_read_unlock();
return &rt->dst;
route_err:
ip_rt_put(rt);
no_route:
- rcu_read_unlock();
__IP_INC_STATS(net, IPSTATS_MIB_OUTNOROUTES);
return NULL;
}
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index b1bb1b3a108232d56aa82383422d68b5ff9da3ed..77cf32a80952fcf3ceff4ada946cc2d0df2411d9 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -355,7 +355,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
/* We throwed the options of the initial SYN away, so we hope
* the ACK carries the same options again (see RFC1122 4.2.3.8)
*/
- ireq->opt = tcp_v4_save_options(sock_net(sk), skb);
+ RCU_INIT_POINTER(ireq->ireq_opt, tcp_v4_save_options(sock_net(sk), skb));
if (security_inet_conn_request(sk, skb, req)) {
reqsk_free(req);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c5d7656beeee29b3c92e1c8824dbf00d3fa32d28..7eec3383702bbab497a12095b55d255532ad5f60 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6196,7 +6196,7 @@ struct request_sock *inet_reqsk_alloc(const struct request_sock_ops *ops,
struct inet_request_sock *ireq = inet_rsk(req);
kmemcheck_annotate_bitfield(ireq, flags);
- ireq->opt = NULL;
+ ireq->ireq_opt = NULL;
#if IS_ENABLED(CONFIG_IPV6)
ireq->pktopts = NULL;
#endif
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 85164d4d3e537537c87d74c00172592c860d4dfb..4c43365c374c8bf868fc0b862333244ca26d5016 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -877,7 +877,7 @@ static int tcp_v4_send_synack(const struct sock *sk, struct dst_entry *dst,
err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
ireq->ir_rmt_addr,
- ireq->opt);
+ rcu_dereference(ireq->ireq_opt));
err = net_xmit_eval(err);
}
@@ -889,7 +889,7 @@ static int tcp_v4_send_synack(const struct sock *sk, struct dst_entry *dst,
*/
static void tcp_v4_reqsk_destructor(struct request_sock *req)
{
- kfree(inet_rsk(req)->opt);
+ kfree(rcu_dereference_protected(inet_rsk(req)->ireq_opt, 1));
}
#ifdef CONFIG_TCP_MD5SIG
@@ -1265,10 +1265,11 @@ static void tcp_v4_init_req(struct request_sock *req,
struct sk_buff *skb)
{
struct inet_request_sock *ireq = inet_rsk(req);
+ struct net *net = sock_net(sk_listener);
sk_rcv_saddr_set(req_to_sk(req), ip_hdr(skb)->daddr);
sk_daddr_set(req_to_sk(req), ip_hdr(skb)->saddr);
- ireq->opt = tcp_v4_save_options(sock_net(sk_listener), skb);
+ RCU_INIT_POINTER(ireq->ireq_opt, tcp_v4_save_options(net, skb));
}
static struct dst_entry *tcp_v4_route_req(const struct sock *sk,
@@ -1355,10 +1356,9 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
sk_daddr_set(newsk, ireq->ir_rmt_addr);
sk_rcv_saddr_set(newsk, ireq->ir_loc_addr);
newsk->sk_bound_dev_if = ireq->ir_iif;
- newinet->inet_saddr = ireq->ir_loc_addr;
- inet_opt = ireq->opt;
- rcu_assign_pointer(newinet->inet_opt, inet_opt);
- ireq->opt = NULL;
+ newinet->inet_saddr = ireq->ir_loc_addr;
+ inet_opt = rcu_dereference(ireq->ireq_opt);
+ RCU_INIT_POINTER(newinet->inet_opt, inet_opt);
newinet->mc_index = inet_iif(skb);
newinet->mc_ttl = ip_hdr(skb)->ttl;
newinet->rcv_tos = ip_hdr(skb)->tos;
@@ -1403,9 +1403,12 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
if (__inet_inherit_port(sk, newsk) < 0)
goto put_and_exit;
*own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash));
- if (*own_req)
+ if (likely(*own_req)) {
tcp_move_syn(newtp, req);
-
+ ireq->ireq_opt = NULL;
+ } else {
+ newinet->inet_opt = NULL;
+ }
return newsk;
exit_overflow:
@@ -1416,6 +1419,7 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
tcp_listendrop(sk);
return NULL;
put_and_exit:
+ newinet->inet_opt = NULL;
inet_csk_prepare_forced_close(newsk);
tcp_done(newsk);
goto exit;
^ permalink raw reply related
* Re: [PATCH net-next 6/8] tools: bpftool: print all relevant byte opcodes for "load double word"
From: Daniel Borkmann @ 2017-10-20 16:12 UTC (permalink / raw)
To: Quentin Monnet
Cc: David Laight, Jakub Kicinski, netdev@vger.kernel.org,
oss-drivers@netronome.com
In-Reply-To: <CAH3iqT5vPnOCEm6wjavbpSyC4Md922Bts3ynxxVCT-1=MWsgKA@mail.gmail.com>
On 10/20/2017 05:50 PM, Quentin Monnet wrote:
[...]
> The remaining four bytes are taken from the "immediate" field of the second
> instruction, which leaves the first four fields (offset, source and destination
> registers, and in particular opcode) unused. As far as I know, these fields
> remain at zero, and this makes it the only “instruction” to have a null code
> (although I am not sure this is a strict requirement, because I did not find
> the code in the verifier that would reject a program having a non-null opcode
> right after a "load double word immediate" instruction).
It's in replace_map_fd_with_map_ptr(), invalid insns for the 2nd part
are rejected there, they have to be otherwise it's not extendable anymore
from abi pov; check also 'test1* ld_imm64' in the verifier test cases.
Cheers,
Daniel
^ permalink raw reply
* Re: [PATCH 54/58] net/xen-netback: Convert timers to use timer_setup()
From: Wei Liu @ 2017-10-20 16:16 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Wei Liu, Paul Durrant, xen-devel, netdev,
Thomas Gleixner, linux-kernel
In-Reply-To: <1508200182-104605-55-git-send-email-keescook@chromium.org>
On Mon, Oct 16, 2017 at 05:29:38PM -0700, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Paul Durrant <paul.durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Wei Liu <wei.liu2@citrix.com>
^ permalink raw reply
* Re: [PATCH 2/2] net: netrom: refactor code in nr_add_node
From: Gustavo A. R. Silva @ 2017-10-20 16:06 UTC (permalink / raw)
To: walter harms; +Cc: Ralf Baechle, David S. Miller, linux-hams, netdev
In-Reply-To: <59E9BA66.4000707@bfs.de>
Hi Walter,
Quoting walter harms <wharms@bfs.de>:
> Am 19.10.2017 19:27, schrieb Gustavo A. R. Silva:
>> Code refactoring in order to make the code easier to read and maintain.
>>
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>> This code was tested by compilation only (GCC 7.2.0 was used).
>>
>> net/netrom/nr_route.c | 63
>> ++++++++++++++++-----------------------------------
>> 1 file changed, 20 insertions(+), 43 deletions(-)
>>
>> diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
>> index fc9cadc..1e5165f 100644
>> --- a/net/netrom/nr_route.c
>> +++ b/net/netrom/nr_route.c
>> @@ -80,6 +80,23 @@ static struct nr_neigh
>> *nr_neigh_get_dev(ax25_address *callsign,
>>
>> static void nr_remove_neigh(struct nr_neigh *);
>>
>> +/* re-sort the routes in quality order. */
>> +static inline void re_sort_routes(struct nr_node *nr_node, int
>> ix_x, int ix_y)
>> +{
>> + struct nr_route nr_route;
>> +
>> + if (nr_node->routes[ix_y].quality > nr_node->routes[ix_x].quality) {
>> + if (nr_node->which == ix_x)
>> + nr_node->which = ix_y;
>> + else if (nr_node->which == ix_y)
>> + nr_node->which = ix_x;
>> +
>> + nr_route = nr_node->routes[ix_x];
>> + nr_node->routes[ix_x] = nr_node->routes[ix_y];
>> + nr_node->routes[ix_y] = nr_route;
>> + }
>> +}
>> +
>
>
> Good idea, a bit of nit picking ..
> does ix_ has a special meaning ? otherwise x,y would be sufficient.
ix typical stands for index, but I think just x and y are fine too.
> From the code below i can see: y=x+1 Perhaps that can be used.
>
So are you proposing to use two arguments instead of three?
re_sort_routes(nr_node, 0);
> kernel.h has a swap() macro. so you can
> swap(nr_node->routes[x],nr_node->routes[y]);
>
Nice, I will use that macro.
> hope that helps,
Definitely. I appreciate your comments.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next 6/8] tools: bpftool: print all relevant byte opcodes for "load double word"
From: Quentin Monnet @ 2017-10-20 16:41 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David Laight, Jakub Kicinski, netdev@vger.kernel.org,
oss-drivers@netronome.com
In-Reply-To: <59EA2077.7000901@iogearbox.net>
On 20 October 2017 at 17:12, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 10/20/2017 05:50 PM, Quentin Monnet wrote:
> [...]
>>
>> The remaining four bytes are taken from the "immediate" field of the
>> second
>> instruction, which leaves the first four fields (offset, source and
>> destination
>> registers, and in particular opcode) unused. As far as I know, these
>> fields
>> remain at zero, and this makes it the only “instruction” to have a null
>> code
>> (although I am not sure this is a strict requirement, because I did not
>> find
>> the code in the verifier that would reject a program having a non-null
>> opcode
>> right after a "load double word immediate" instruction).
>
>
> It's in replace_map_fd_with_map_ptr(), invalid insns for the 2nd part
> are rejected there, they have to be otherwise it's not extendable anymore
> from abi pov; check also 'test1* ld_imm64' in the verifier test cases.
>
> Cheers,
> Daniel
Indeed, thanks Daniel!
Quentin
^ permalink raw reply
* Re: [PATCH 18/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: jlee @ 2017-10-20 16:43 UTC (permalink / raw)
To: David Howells
Cc: Alexei Starovoitov, linux-security-module, gnomes, linux-efi,
matthew.garrett, gregkh, linux-kernel, jforbes, Daniel Borkmann,
David S. Miller, netdev, Gary Lin
In-Reply-To: <19424.1508515402@warthog.procyon.org.uk>
On Fri, Oct 20, 2017 at 05:03:22PM +0100, David Howells wrote:
> jlee@suse.com wrote:
>
> > I think that we don't need to lock down sys_bpf() now because
> > we didn't lock down other interfaces for reading arbitrary
> > address like /dev/mem and /dev/kmem.
>
> Ummm... See patch 4. You even gave me a Reviewed-by for it ;-)
>
> David
hm... patch 4 only prevents write_mem() but not read_mem().
Or I missed anything?
Thanks
Joey Lee
^ permalink raw reply
* [PATCH] net: sunrpc: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-20 16:48 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, J. Bruce Fields, Jeff Layton,
David S. Miller
Cc: linux-nfs, netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/sunrpc/clnt.c | 11 +++++++++++
net/sunrpc/xprt.c | 1 +
net/sunrpc/xprtsock.c | 4 ++++
3 files changed, 16 insertions(+)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 2ad827d..d25b077 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1586,6 +1586,7 @@ call_reserveresult(struct rpc_task *task)
switch (status) {
case -ENOMEM:
rpc_delay(task, HZ >> 2);
+ /* fall through */
case -EAGAIN: /* woken up; retry */
task->tk_action = call_retry_reserve;
return;
@@ -1647,10 +1648,13 @@ call_refreshresult(struct rpc_task *task)
/* Use rate-limiting and a max number of retries if refresh
* had status 0 but failed to update the cred.
*/
+ /* fall through */
case -ETIMEDOUT:
rpc_delay(task, 3*HZ);
+ /* fall through */
case -EAGAIN:
status = -EACCES;
+ /* fall through */
case -EKEYEXPIRED:
if (!task->tk_cred_retry)
break;
@@ -1911,6 +1915,7 @@ call_connect_status(struct rpc_task *task)
task->tk_action = call_bind;
return;
}
+ /* fall through */
case -ECONNRESET:
case -ECONNABORTED:
case -ENETUNREACH:
@@ -1924,6 +1929,7 @@ call_connect_status(struct rpc_task *task)
break;
/* retry with existing socket, after a delay */
rpc_delay(task, 3*HZ);
+ /* fall through */
case -EAGAIN:
/* Check for timeouts before looping back to call_bind */
case -ETIMEDOUT:
@@ -2025,6 +2031,7 @@ call_transmit_status(struct rpc_task *task)
rpc_exit(task, task->tk_status);
break;
}
+ /* fall through */
case -ECONNRESET:
case -ECONNABORTED:
case -EADDRINUSE:
@@ -2145,6 +2152,7 @@ call_status(struct rpc_task *task)
* were a timeout.
*/
rpc_delay(task, 3*HZ);
+ /* fall through */
case -ETIMEDOUT:
task->tk_action = call_timeout;
break;
@@ -2152,14 +2160,17 @@ call_status(struct rpc_task *task)
case -ECONNRESET:
case -ECONNABORTED:
rpc_force_rebind(clnt);
+ /* fall through */
case -EADDRINUSE:
rpc_delay(task, 3*HZ);
+ /* fall through */
case -EPIPE:
case -ENOTCONN:
task->tk_action = call_bind;
break;
case -ENOBUFS:
rpc_delay(task, HZ>>2);
+ /* fall through */
case -EAGAIN:
task->tk_action = call_transmit;
break;
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 1a39ad1..197883f 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1139,6 +1139,7 @@ void xprt_alloc_slot(struct rpc_xprt *xprt, struct rpc_task *task)
case -EAGAIN:
xprt_add_backlog(xprt, task);
dprintk("RPC: waiting for request slot\n");
+ /* fall through */
default:
task->tk_status = -EAGAIN;
}
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index c1841f2..684e356 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -551,6 +551,7 @@ static int xs_local_send_request(struct rpc_task *task)
default:
dprintk("RPC: sendmsg returned unrecognized error %d\n",
-status);
+ /* fall through */
case -EPIPE:
xs_close(xprt);
status = -ENOTCONN;
@@ -1610,6 +1611,7 @@ static void xs_tcp_state_change(struct sock *sk)
xprt->connect_cookie++;
clear_bit(XPRT_CONNECTED, &xprt->state);
xs_tcp_force_close(xprt);
+ /* fall through */
case TCP_CLOSING:
/*
* If the server closed down the connection, make sure that
@@ -2367,6 +2369,7 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
switch (ret) {
case 0:
xs_set_srcport(transport, sock);
+ /* fall through */
case -EINPROGRESS:
/* SYN_SENT! */
if (xprt->reestablish_timeout < XS_TCP_INIT_REEST_TO)
@@ -2418,6 +2421,7 @@ static void xs_tcp_setup_socket(struct work_struct *work)
default:
printk("%s: connect returned unhandled error %d\n",
__func__, status);
+ /* fall through */
case -EADDRNOTAVAIL:
/* We're probably in TIME_WAIT. Get rid of existing socket,
* and retry
--
2.7.4
^ 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