1. 27 Apr, 2019 14 commits
    • Johannes Berg's avatar
      genetlink: optionally validate strictly/dumps · ef6243ac
      Johannes Berg authored
      Add options to strictly validate messages and dump messages,
      sometimes perhaps validating dump messages non-strictly may
      be required, so add an option for that as well.
      
      Since none of this can really be applied to existing commands,
      set the options everwhere using the following spatch:
      
          @@
          identifier ops;
          expression X;
          @@
          struct genl_ops ops[] = {
          ...,
           {
                  .cmd = X,
          +       .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
                  ...
           },
          ...
          };
      
      For new commands one should just not copy the .validate 'opt-out'
      flags and thus get strict validation.
      Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ef6243ac
    • Johannes Berg's avatar
      netlink: add strict parsing for future attributes · 56738f46
      Johannes Berg authored
      Unfortunately, we cannot add strict parsing for all attributes, as
      that would break existing userspace. We currently warn about it, but
      that's about all we can do.
      
      For new attributes, however, the story is better: nobody is using
      them, so we can reject bad sizes.
      
      Also, for new attributes, we need not accept them when the policy
      doesn't declare their usage.
      
      David Ahern and I went back and forth on how to best encode this, and
      the best way we found was to have a "boundary type", from which point
      on new attributes have all possible validation applied, and NLA_UNSPEC
      is rejected.
      
      As we didn't want to add another argument to all functions that get a
      netlink policy, the workaround is to encode that boundary in the first
      entry of the policy array (which is for type 0 and thus probably not
      really valid anyway). I put it into the validation union for the rare
      possibility that somebody is actually using attribute 0, which would
      continue to work fine unless they tried to use the extended validation,
      which isn't likely. We also didn't find any in-tree users with type 0.
      
      The reason for setting the "start strict here" attribute is that we
      never really need to start strict from 0, which is invalid anyway (or
      in legacy families where that isn't true, it cannot be set to strict),
      so we can thus reserve the value 0 for "don't do this check" and don't
      have to add the tag to all policies right now.
      
      Thus, policies can now opt in to this validation, which we should do
      for all existing policies, at least when adding new attributes.
      
      Note that entirely *new* policies won't need to set it, as the use
      of that should be using nla_parse()/nlmsg_parse() etc. which anyway
      do fully strict validation now, regardless of this.
      
      So in effect, this patch only covers the "existing command with new
      attribute" case.
      Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      56738f46
    • Johannes Berg's avatar
      netlink: re-add parse/validate functions in strict mode · 3de64403
      Johannes Berg authored
      This re-adds the parse and validate functions like nla_parse()
      that are now actually strict after the previous rename and were
      just split out to make sure everything is converted (and if not
      compilation of the previous patch would fail.)
      Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      3de64403
    • Johannes Berg's avatar
      netlink: make validation more configurable for future strictness · 8cb08174
      Johannes Berg authored
      We currently have two levels of strict validation:
      
       1) liberal (default)
           - undefined (type >= max) & NLA_UNSPEC attributes accepted
           - attribute length >= expected accepted
           - garbage at end of message accepted
       2) strict (opt-in)
           - NLA_UNSPEC attributes accepted
           - attribute length >= expected accepted
      
      Split out parsing strictness into four different options:
       * TRAILING     - check that there's no trailing data after parsing
                        attributes (in message or nested)
       * MAXTYPE      - reject attrs > max known type
       * UNSPEC       - reject attributes with NLA_UNSPEC policy entries
       * STRICT_ATTRS - strictly validate attribute size
      
      The default for future things should be *everything*.
      The current *_strict() is a combination of TRAILING and MAXTYPE,
      and is renamed to _deprecated_strict().
      The current regular parsing has none of this, and is renamed to
      *_parse_deprecated().
      
      Additionally it allows us to selectively set one of the new flags
      even on old policies. Notably, the UNSPEC flag could be useful in
      this case, since it can be arranged (by filling in the policy) to
      not be an incompatible userspace ABI change, but would then going
      forward prevent forgetting attribute entries. Similar can apply
      to the POLICY flag.
      
      We end up with the following renames:
       * nla_parse           -> nla_parse_deprecated
       * nla_parse_strict    -> nla_parse_deprecated_strict
       * nlmsg_parse         -> nlmsg_parse_deprecated
       * nlmsg_parse_strict  -> nlmsg_parse_deprecated_strict
       * nla_parse_nested    -> nla_parse_nested_deprecated
       * nla_validate_nested -> nla_validate_nested_deprecated
      
      Using spatch, of course:
          @@
          expression TB, MAX, HEAD, LEN, POL, EXT;
          @@
          -nla_parse(TB, MAX, HEAD, LEN, POL, EXT)
          +nla_parse_deprecated(TB, MAX, HEAD, LEN, POL, EXT)
      
          @@
          expression NLH, HDRLEN, TB, MAX, POL, EXT;
          @@
          -nlmsg_parse(NLH, HDRLEN, TB, MAX, POL, EXT)
          +nlmsg_parse_deprecated(NLH, HDRLEN, TB, MAX, POL, EXT)
      
          @@
          expression NLH, HDRLEN, TB, MAX, POL, EXT;
          @@
          -nlmsg_parse_strict(NLH, HDRLEN, TB, MAX, POL, EXT)
          +nlmsg_parse_deprecated_strict(NLH, HDRLEN, TB, MAX, POL, EXT)
      
          @@
          expression TB, MAX, NLA, POL, EXT;
          @@
          -nla_parse_nested(TB, MAX, NLA, POL, EXT)
          +nla_parse_nested_deprecated(TB, MAX, NLA, POL, EXT)
      
          @@
          expression START, MAX, POL, EXT;
          @@
          -nla_validate_nested(START, MAX, POL, EXT)
          +nla_validate_nested_deprecated(START, MAX, POL, EXT)
      
          @@
          expression NLH, HDRLEN, MAX, POL, EXT;
          @@
          -nlmsg_validate(NLH, HDRLEN, MAX, POL, EXT)
          +nlmsg_validate_deprecated(NLH, HDRLEN, MAX, POL, EXT)
      
      For this patch, don't actually add the strict, non-renamed versions
      yet so that it breaks compile if I get it wrong.
      
      Also, while at it, make nla_validate and nla_parse go down to a
      common __nla_validate_parse() function to avoid code duplication.
      
      Ultimately, this allows us to have very strict validation for every
      new caller of nla_parse()/nlmsg_parse() etc as re-introduced in the
      next patch, while existing things will continue to work as is.
      
      In effect then, this adds fully strict validation for any new command.
      Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      8cb08174
    • Johannes Berg's avatar
      netlink: add NLA_MIN_LEN · 6f455f5f
      Johannes Berg authored
      Rather than using NLA_UNSPEC for this type of thing, use NLA_MIN_LEN
      so we can make NLA_UNSPEC be NLA_REJECT under certain conditions for
      future attributes.
      
      While at it, also use NLA_EXACT_LEN for the struct example.
      Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6f455f5f
    • David S. Miller's avatar
      Merge branch 'nla_nest_start' · f6ad55a6
      David S. Miller authored
      Michal Kubecek says:
      
      ====================
      make nla_nest_start() add NLA_F_NESTED flag
      
      One of the comments in recent review of the ethtool netlink series pointed
      out that proposed ethnl_nest_start() helper which adds NLA_F_NESTED to
      second argument of nla_nest_start() is not really specific to ethtool
      netlink code. That is hard to argue with as closer inspection revealed that
      exactly the same helper already exists in ipset code (except it's a macro
      rather than an inline function).
      
      Another observation was that even if NLA_F_NESTED flag was introduced in
      2007, only few netlink based interfaces set it in kernel generated messages
      and even many recently added APIs omit it. That is unfortunate as without
      the flag, message parsers not familiar with attribute semantics cannot
      recognize nested attributes and do not see message structure; this affects
      e.g. wireshark dissector or mnl_nlmsg_fprintf() from libmnl.
      
      This is why I'm suggesting to rename existing nla_nest_start() to different
      name (nla_nest_start_noflag) and reintroduce nla_nest_start() as a wrapper
      adding NLA_F_NESTED flag. This is implemented in first patch which is
      mostly generated by spatch. Second patch drops ipset helper macros which
      lose their purpose. Third patch cleans up minor coding style issues found
      by checkpatch.pl in first patch.
      
      We could leave nla_nest_start() untouched and simply add a wrapper adding
      NLA_F_NESTED but that would probably preserve the state when even most new
      code doesn't set the flag.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f6ad55a6
    • Michal Kubecek's avatar
      net: fix two coding style issues · f78c6032
      Michal Kubecek authored
      This is a simple cleanup addressing two coding style issues found by
      checkpatch.pl in an earlier patch. It's submitted as a separate patch to
      keep the original patch as it was generated by spatch.
      Signed-off-by: default avatarMichal Kubecek <mkubecek@suse.cz>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f78c6032
    • Michal Kubecek's avatar
      ipset: drop ipset_nest_start() and ipset_nest_end() · 12ad5f65
      Michal Kubecek authored
      After the previous commit, both ipset_nest_start() and ipset_nest_end() are
      just aliases for nla_nest_start() and nla_nest_end() so that there is no
      need to keep them.
      Signed-off-by: default avatarMichal Kubecek <mkubecek@suse.cz>
      Acked-by: default avatarJozsef Kadlecsik <kadlec@blackhole.kfki.hu>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      12ad5f65
    • Michal Kubecek's avatar
      netlink: make nla_nest_start() add NLA_F_NESTED flag · ae0be8de
      Michal Kubecek authored
      Even if the NLA_F_NESTED flag was introduced more than 11 years ago, most
      netlink based interfaces (including recently added ones) are still not
      setting it in kernel generated messages. Without the flag, message parsers
      not aware of attribute semantics (e.g. wireshark dissector or libmnl's
      mnl_nlmsg_fprintf()) cannot recognize nested attributes and won't display
      the structure of their contents.
      
      Unfortunately we cannot just add the flag everywhere as there may be
      userspace applications which check nlattr::nla_type directly rather than
      through a helper masking out the flags. Therefore the patch renames
      nla_nest_start() to nla_nest_start_noflag() and introduces nla_nest_start()
      as a wrapper adding NLA_F_NESTED. The calls which add NLA_F_NESTED manually
      are rewritten to use nla_nest_start().
      
      Except for changes in include/net/netlink.h, the patch was generated using
      this semantic patch:
      
      @@ expression E1, E2; @@
      -nla_nest_start(E1, E2)
      +nla_nest_start_noflag(E1, E2)
      
      @@ expression E1, E2; @@
      -nla_nest_start_noflag(E1, E2 | NLA_F_NESTED)
      +nla_nest_start(E1, E2)
      Signed-off-by: default avatarMichal Kubecek <mkubecek@suse.cz>
      Acked-by: default avatarJiri Pirko <jiri@mellanox.com>
      Acked-by: default avatarDavid Ahern <dsahern@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ae0be8de
    • David S. Miller's avatar
      Merge branch 'net-tls-small-code-cleanup' · c7881b4a
      David S. Miller authored
      Jakub Kicinski says:
      
      ====================
      net/tls: small code cleanup
      
      This small patch set cleans up tls (mostly offload parts).
      Other than avoiding unnecessary error messages - no functional
      changes here.
      
      v2 (Saeed):
       - fix up Review tags;
       - remove the warning on failure completely.
      ====================
      Reviewed-by: default avatarSaeed Mahameed <saeedm@mellanox.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      c7881b4a
    • Jakub Kicinski's avatar
      net/tls: byte swap device req TCP seq no upon setting · 63a1c95f
      Jakub Kicinski authored
      To avoid a sparse warning byteswap the be32 sequence number
      before it's stored in the atomic value.  While at it drop
      unnecessary brackets and use kernel's u64 type.
      Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@netronome.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      63a1c95f
    • Jakub Kicinski's avatar
      net/tls: move definition of tls ops into net/tls.h · da68b4ad
      Jakub Kicinski authored
      There seems to be no reason for tls_ops to be defined in netdevice.h
      which is included in a lot of places.  Don't wrap the struct/enum
      declaration in ifdefs, it trickles down unnecessary ifdefs into
      driver code.
      Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@netronome.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      da68b4ad
    • Jakub Kicinski's avatar
      net/tls: remove old exports of sk_destruct functions · 9e995797
      Jakub Kicinski authored
      tls_device_sk_destruct being set on a socket used to indicate
      that socket is a kTLS device one.  That is no longer true -
      now we use sk_validate_xmit_skb pointer for that purpose.
      Remove the export.  tls_device_attach() needs to be moved.
      
      While at it, remove the dead declaration of tls_sk_destruct().
      Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
      Reviewed-by: default avatarDirk van der Merwe <dirk.vandermerwe@netronome.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@netronome.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9e995797
    • Jakub Kicinski's avatar
      net/tls: don't log errors every time offload can't proceed · e49d268d
      Jakub Kicinski authored
      Currently when CONFIG_TLS_DEVICE is set each time kTLS
      connection is opened and the offload is not successful
      (either because the underlying device doesn't support
      it or e.g. it's tables are full) a rate limited error
      will be printed to the logs.
      
      There is nothing wrong with failing TLS offload.  SW
      path will process the packets just fine, drop the
      noisy messages.
      Signed-off-by: default avatarJakub Kicinski <jakub.kicinski@netronome.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@netronome.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e49d268d
  2. 26 Apr, 2019 26 commits