netdev - Re: [PATCH 1/2] net: store KCOV remote handle in sk

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, 13 Oct 2020 18:59:28 +0300
From:   Aleksandr Nogikh <a.nogikh@...il.com>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Johannes Berg <johannes@...solutions.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Marco Elver <elver@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        Aleksandr Nogikh <nogikh@...gle.com>
Subject: Re: [PATCH 1/2] net: store KCOV remote handle in sk_buff

On Mon, 12 Oct 2020 at 09:04, Dmitry Vyukov <dvyukov@...gle.com> wrote:
>
> On Sat, Oct 10, 2020 at 5:14 PM Jakub Kicinski <kuba@...nel.org> wrote:
> >
> > On Sat, 10 Oct 2020 09:54:57 +0200 Dmitry Vyukov wrote:
> > > On Sat, Oct 10, 2020 at 1:16 AM Jakub Kicinski <kuba@...nel.org> wrote:
[...]
> > > > Could you use skb_extensions for this?
> > >
> > > Why? If for space, this is already under a non-production ifdef.
> >
> > I understand, but the skb_ext infra is there for uncommon use cases
> > like this one. Any particular reason you don't want to use it?
> > The slight LoC increase?
> >
> > Is there any precedent for adding the kcov field to other performance
> > critical structures?

It would be great to come to some conclusion on where exactly to store
kcov_handle. Technically, it is possible to use skb extensions for the
purpose, though it will indeed slightly increase the complexity.

Jakub, you think that kcov_handle should be added as an skb extension,
right?

Though I do not really object to moving the field, it still seems to
me that sk_buff itself is a better place. Right now skb extensions
store values that are local to specific protocols and that are only
meaningful in the context of these protocols (correct me if I'm
wrong). Although this patch only adds remote kcov coverage to the wifi
code, kcov_handle can be meaningful for other protocols as well - just
like the already existing sk_buff fields. So adding kcov_handle to skb
extensions will break this logical separation.

--
Best regards,
Aleksandr