lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 28 Feb 2017 14:46:20 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Davide Caratti <dcaratti@...hat.com>
Cc:     David Laight <David.Laight@...lab.com>,
        Tom Herbert <tom@...bertland.com>,
        "David S . Miller" <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        "linux-sctp @ vger . kernel . org" <linux-sctp@...r.kernel.org>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Subject: Re: [PATCH RFC net-next v2 1/4] skbuff: add stub to help computing
 crc32c on SCTP packets

On Tue, Feb 28, 2017 at 2:32 AM, Davide Caratti <dcaratti@...hat.com> wrote:
> sctp_compute_checksum requires crc32c symbol (provided by libcrc32c), so
> it can't be used in net core. Like it has been done previously with other
> symbols (e.g. ipv6_dst_lookup), introduce a stub struct skb_checksum_ops
> to allow computation of SCTP checksum in net core after sctp.ko (and thus
> libcrc32c) has been loaded.

At a minimum the name really needs to change.  SCTP does not do
checksums.  It does a CRC, and a CRC is a very different thing.  The
fact that somebody decided that offloading a CRC could use the same
framework is very unfortunate, and your patch descriptions in this
whole set are calling out a CRC as checksums which it is not.

I don't want to see anything "checksum" or "csum" related in the
naming when it comes to dealing with SCTP unless we absolutely have to
have it.  So any function names or structures with sctp in the name
should call out "crc32" or "crc", please don't use checksum.

> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> Signed-off-by: Davide Caratti <dcaratti@...hat.com>
> ---
>  include/linux/skbuff.h |  2 ++
>  net/core/skbuff.c      | 20 ++++++++++++++++++++
>  net/sctp/offload.c     |  7 +++++++
>  3 files changed, 29 insertions(+)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 69ccd26..cab9a32 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3125,6 +3125,8 @@ struct skb_checksum_ops {
>         __wsum (*combine)(__wsum csum, __wsum csum2, int offset, int len);
>  };
>
> +extern const struct skb_checksum_ops *sctp_csum_stub __read_mostly;
> +
>  __wsum __skb_checksum(const struct sk_buff *skb, int offset, int len,
>                       __wsum csum, const struct skb_checksum_ops *ops);
>  __wsum skb_checksum(const struct sk_buff *skb, int offset, int len,
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index f355795..64fd8fd 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2242,6 +2242,26 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
>  }
>  EXPORT_SYMBOL(skb_copy_and_csum_bits);
>
> +static __wsum warn_sctp_csum_update(const void *buff, int len, __wsum sum)
> +{
> +       net_warn_ratelimited("attempt to compute crc32c without sctp.ko\n");
> +       return 0;
> +}
> +
> +static __wsum warn_sctp_csum_combine(__wsum csum, __wsum csum2,
> +                                    int offset, int len)
> +{
> +       net_warn_ratelimited("attempt to compute crc32c without sctp.ko\n");
> +       return 0;
> +}
> +
> +const struct skb_checksum_ops *sctp_csum_stub __read_mostly =
> +       &(struct skb_checksum_ops) {
> +       .update  = warn_sctp_csum_update,
> +       .combine = warn_sctp_csum_combine,
> +};
> +EXPORT_SYMBOL(sctp_csum_stub);
> +
>   /**
>   *     skb_zerocopy_headlen - Calculate headroom needed for skb_zerocopy()
>   *     @from: source buffer
> diff --git a/net/sctp/offload.c b/net/sctp/offload.c
> index 4f5a2b5..e9c3db0 100644
> --- a/net/sctp/offload.c
> +++ b/net/sctp/offload.c
> @@ -98,6 +98,12 @@ static const struct net_offload sctp6_offload = {
>         },
>  };
>
> +static const struct skb_checksum_ops *sctp_csum_ops __read_mostly =
> +       &(struct skb_checksum_ops) {
> +       .update  = sctp_csum_update,
> +       .combine = sctp_csum_combine,
> +};
> +
>  int __init sctp_offload_init(void)
>  {
>         int ret;
> @@ -110,6 +116,7 @@ int __init sctp_offload_init(void)
>         if (ret)
>                 goto ipv4;
>
> +       sctp_csum_stub = sctp_csum_ops;
>         return ret;
>
>  ipv4:
> --
> 2.7.4
>

Powered by blists - more mailing lists