netdev - Re: [PATCH ethtool] ethtool: dump nested registers

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]

Message-ID: <20190809172156.GI31971@unicorn.suse.cz>
Date:   Fri, 9 Aug 2019 19:21:56 +0200
From:   Michal Kubecek <mkubecek@...e.cz>
To:     netdev@...r.kernel.org
Cc:     "John W. Linville" <linville@...driver.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        f.fainelli@...il.com, andrew@...n.ch, davem@...emloft.net,
        linville@...hat.com, cphealy@...il.com
Subject: Re: [PATCH ethtool] ethtool: dump nested registers

On Fri, Aug 09, 2019 at 12:23:36PM -0400, John W. Linville wrote:
> On Fri, Aug 02, 2019 at 03:34:54PM -0400, Vivien Didelot wrote:
> > Usually kernel drivers set the regs->len value to the same length as
> > info->regdump_len, which was used for the allocation. In case where
> > regs->len is smaller than the allocated info->regdump_len length,
> > we may assume that the dump contains a nested set of registers.
> > 
> > This becomes handy for kernel drivers to expose registers of an
> > underlying network conduit unfortunately not exposed to userspace,
> > as found in network switching equipment for example.
> > 
> > This patch adds support for recursing into the dump operation if there
> > is enough room for a nested ethtool_drvinfo structure containing a
> > valid driver name, followed by a ethtool_regs structure like this:
> > 
> >     0      regs->len                        info->regdump_len
> >     v              v                                        v
> >     +--------------+-----------------+--------------+-- - --+
> >     | ethtool_regs | ethtool_drvinfo | ethtool_regs |       |
> >     +--------------+-----------------+--------------+-- - --+
> > 
> > Signed-off-by: Vivien Didelot <vivien.didelot@...il.com>
> 
> I wasn't sure what to do with this one, given the disucssion that
> followed. But since Dave merged "net: dsa: dump CPU port regs through
> master" for net-next, I went ahead and queued this one for the next
> release. If that was the wrong thing to do, speak-up now!

I'm certainly not happy about it as I'm afraid it's going to give me
a headache one day. But as long as the driver packs CPU port registers
into the device's register dump, it doesn't make sense not to allow
ethtool to parse them. And I'm not sure my objections are strong enough
to request a revert of the kernel part as I'm not sure the idea of
transforming the register dump into a structured format (an actual list
of registers rather than an opaque data block) is feasible at all.

So let's keep the patch in.

Michal