netdev - Re: [PATCH net v2 3/4] ipv6: no CHECKSUM_PARTIAL on MSG

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]

Message-ID: <CALx6S37F236V6YxMhHGbZNCsXxHTTfeXpAXj99GMyA3hnyH5XA@mail.gmail.com>
Date:	Tue, 27 Oct 2015 15:03:49 -0700
From:	Tom Herbert <tom@...bertland.com>
To:	Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	Vladislav Yasevich <vyasevich@...il.com>,
	Benjamin Coddington <bcodding@...hat.com>
Subject: Re: [PATCH net v2 3/4] ipv6: no CHECKSUM_PARTIAL on MSG_MORE corked sockets

On Tue, Oct 27, 2015 at 2:42 PM, Hannes Frederic Sowa
<hannes@...essinduktion.org> wrote:
> Hi Tom,
>
> On Tue, Oct 27, 2015, at 20:19, Hannes Frederic Sowa wrote:
>> On Tue, Oct 27, 2015, at 19:37, Tom Herbert wrote:
>> > On Tue, Oct 27, 2015 at 11:29 AM, Hannes Frederic Sowa
>> > <hannes@...essinduktion.org> wrote:
>> > > On Tue, Oct 27, 2015, at 18:32, Tom Herbert wrote:
>> > >> On Tue, Oct 27, 2015 at 9:44 AM, Hannes Frederic Sowa
>> > >> <hannes@...essinduktion.org> wrote:
>> > >> >
>> > >> >
>> > >> > On Tue, Oct 27, 2015, at 17:36, Tom Herbert wrote:> > -               if
>> > >> > (cork->length + length > maxnonfragsize - headersize) {
>> > >> >> > +       if (cork->length + length > maxnonfragsize - headersize) {
>> > >> >> >  emsgsize:
>> > >> >> > -                       ipv6_local_error(sk, EMSGSIZE, fl6,
>> > >> >> > -                                        mtu - headersize +
>> > >> >> > -                                        sizeof(struct ipv6hdr));
>> > >> >> > -                       return -EMSGSIZE;
>> > >> >> > -               }
>> > >> >> > +               ipv6_local_error(sk, EMSGSIZE, fl6,
>> > >> >> > +                                mtu - headersize +
>> > >> >> > +                                sizeof(struct ipv6hdr));
>> > >> >> > +               return -EMSGSIZE;
>> > >> >> >         }
>> > >> >> >
>> > >> >> > +       /* CHECKSUM_PARTIAL only with no extension headers and when
>> > >> >>
>> > >> >> No, please don't do this. CHECKSUM_PARTIAL should work with extension
>> > >> >> headers as defined, so this is just disabling otherwise valid and
>> > >> >> useful functionality. If (some) drivers have problems with this they
>> > >> >> need to be identified and fixed.
>> > >> >
>> > >> > I don't understand. The old code already didn't allow the use of
>> > >> > opt_flen with CHECKSUM_PARTIAL.
>> > >> >
>> > >> Then that's a problem with the old code :-). Is there any other reason
>> > >> that we can't use CHECKSUM_PARTIAL with extension headers other than
>> > >> lack of correct driver support?
>> > >
>> > > The lack of correct driver support is a big bumper, but as I wrote, I
>> > > don't see a reason to not lift this restriction in net-next. I proposed
>> > > a new feature flag, or by looking at your series, we could probably use
>> > > the extension header okay field for that.
>> > >
>> > Okay, but why bother doing this for net? This problem has obviously
>> > existed for a while, and even if the restriction is maintained here
>> > there are still other paths that don't go through ip_append_data that
>> > could trip the bug. Also, drivers are welcome to fix their issues in
>> > net I believe.
>>
>> I even don't know if it could be a hardware issue. Also I don't want to
>> break people's communication with a patch.
>> IMHO without the WARN_ON_ONCEs, which I agreed to remove, I currently
>> don't see any problem for net.
>>
>> You don't agree on a netdev-feature flag, indicating the driver is okay
>> with hardware checksumming and extension headers? We could add this to
>> net-next pretty fast, I think. It does not require people to revert this
>> patch in case their driver misbehaves and we don't get a fix for it,
>> soon. Also what should we do if the driver simply does not support
>> extension headers + checksum offloading? Completely kill checksum
>> offloading for IPv6?
>
> I posted v3 just now. I would like to let David consider it for net
> inclusion. We can work on how to lift this limitation then in net-next,
> okay? I am currently in favor of a new netdev-feature. What do you
> think? Your RFC series could help here, too.
>
I really do not like the feature flag, it's just a bandaid over the
real problem-- in fact my goal is to eliminate NETIF_F_IP{V6}_CSUM and
just have NETIF_F_HW_CSUM. I will repost the helper patches, but we
really do need to start fixing this stuff in the drivers instead of
more hacking in the stack.

Tom

> Thanks,
> Hannes
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html