netdev - Re: [PATCH net] net: qed: prevent buffer overflow when collecting debug data

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: <20200703235704.266-1-alobakin@marvell.com>
Date:   Sat, 4 Jul 2020 02:57:04 +0300
From:   Alexander Lobakin <alobakin@...vell.com>
To:     David Miller <davem@...emloft.net>
CC:     Alexander Lobakin <alobakin@...vell.com>, <kuba@...nel.org>,
        <irusskikh@...vell.com>, <michal.kalderon@...vell.com>,
        <aelior@...vell.com>, <denis.bolotin@...vell.com>,
        <GR-everest-linux-l2@...vell.com>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net] net: qed: prevent buffer overflow when collecting debug data

From:   David Miller <davem@...emloft.net>
Date:   Fri, 03 Jul 2020 12:59:33 -0700 (PDT)

> From: Alexander Lobakin <alobakin@...vell.com>
> Date: Fri, 3 Jul 2020 12:02:58 +0300
> 
> > When generating debug dump, driver firstly collects all data in binary
> > form, and then performs per-feature formatting to human-readable if it
> > is supported.
> > The size of the new formatted data is often larger than the raw's. This
> > becomes critical when user requests dump via ethtool (-d/-w), as output
> > buffer size is strictly determined (by ethtool_ops::get_regs_len() etc),
> > as it may lead to out-of-bounds writes and memory corruption.
> > 
> > To not go past initial lengths, add a flag to return original,
> > non-formatted debug data, and set it in such cases. Also set data type
> > in regdump headers, so userland parsers could handle it.
> > 
> > Fixes: c965db444629 ("qed: Add support for debug data collection")
> > Signed-off-by: Alexander Lobakin <alobakin@...vell.com>
> > Signed-off-by: Igor Russkikh <irusskikh@...vell.com>
> 
> This is now how ethtool register dumps work.
> 
> It does not provide "human readable" versions of register data.  Instead
> it is supposed to be purely raw data and then userland utilities interpret
> that data and can make it human readable based upon the driver name and
> reg dump version.
> 
> Please fix your ethtool -d implementation to comply with this.

This is exactly what this patch does: forces driver to dump raw binary
data. Current mainline version tries to perform formatting before passing
data up to ethtool infra.

> Thank you.

Thanks,
Al