netdev - Re: [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC

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] [day] [month] [year] [list]

Date:   Mon, 22 Aug 2016 08:32:17 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Ben Hutchings <ben@...adent.org.uk>
Cc:     Alexander Duyck <alexander.h.duyck@...el.com>,
        Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [net-next PATCH] ethtool: Add support for SCTP verification tag
 in Rx NFC

On Mon, Aug 22, 2016 at 6:05 AM, Ben Hutchings <ben@...adent.org.uk> wrote:
> On Sat, 2016-08-20 at 18:56 -0700, Alexander Duyck wrote:
>> > On Sat, Aug 20, 2016 at 5:21 PM, Ben Hutchings <ben@...adent.org.uk> wrote:
>> >
>> > On Fri, 2016-08-19 at 14:32 -0700, Alexander Duyck wrote:
>> > >
>> > > The i40e hardware has support for SCTP filtering via Rx NFC however the
>> > > default configuration expects us to include the verification tag as a part
>> > > of the filter.  In order to support that I need to be able to transfer that
>> > > data through the ethtool interface via a new structure.
>> > >
>> > > This patch adds a new structure to allow us to pass the verification tag
>> > > for IPv4 or IPv6 SCTP traffic.
>> > [...]
>> >
>> > This looks like an incompatible ABI change.  I suppose it could be OK
>> > if no drivers implemented flow steering for SCTP using the previously
>> > specified structure, but have you checked that that is the case?
>> >
>> > Ben.
>>
>> Well the structure itself matches the TCP flow spec for the TCP flow
>> sized portion.  All I am doing with this patch is adding an extension
>> to that which is still confined to the 52 byte limit of the flow
>> > union.
>
> But that extension will be ignored by any drivers that implemented the
> API as previously defined.  (If there aren't any, as I said, this
> doesn't really matter.)

The ixgbe driver supports sctp already, and was using the TCP flow
spec to do it.

> With previous extensions (everything in struct ethtool_flow_ext) we've
> introduced new type flags to ensure that they won't be silently
> ignored.  You could add a new extended-SCTP type value for the same
> reason.

I guess I could go the extended flow route.  I an just define a new
type for SCTP_V4 and SCTP_V6.

> [...]
>> One thing I could do if you would like would be to spin up another
>> patch to force the kernel to return -EINVAL if we are masking in
>> fields that are out of bounds for the flow specification.  That way we
>> can handle this  a bit more concisely in the future should we end up
>> > having to extend any other flow specifications.
>
> It's too late to do that now.

I disagree.  For now what I can do is lock down the existing flow
specifications so nobody tries to cheat and smuggle data in on the
unused space and for the new extended specifications we make sure that
they are included in a mask verification so that if we have to add any
new fields they will already be checked for.

I'll make sure to include such a patch for v2.

Thanks.

- Alex