netdev - Re: [PATCH net-next v2 1/2] hisilicon net: removes the once HANDEL_TX

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: <6398202.hrOAl1xuJE@wuerfel>
Date:	Mon, 12 Oct 2015 13:59:39 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	huangdaode <huangdaode@...ilicon.com>
Cc:	davem@...emloft.net, joe@...ches.com, liguozhu@...ilicon.com,
	Yisen.Zhuang@...wei.com, netdev@...r.kernel.org,
	linuxarm@...wei.com, salil.mehta@...wei.com,
	kenneth-lee-2012@...mail.com, xuwei5@...ilicon.com,
	lisheng011@...wei.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/2] hisilicon net: removes the once HANDEL_TX_MSG macro

On Monday 12 October 2015 11:23:44 huangdaode wrote:
> +                       s += sprintf(s,
> +                                   "\t\ttx_ring on %p:%u,%u,%u,%u,%u,%llu,%llu\n",
> +                                   h->qs[i]->tx_ring.io_base,
> +                                   h->qs[i]->tx_ring.buf_size,
> +                                   h->qs[i]->tx_ring.desc_num,
> +                                   h->qs[i]->tx_ring.max_desc_num_per_pkt,
> +                                   h->qs[i]->tx_ring.max_raw_data_sz_per_desc,
> +                                   h->qs[i]->tx_ring.max_pkt_size,
> +                                   h->qs[i]->tx_ring.stats.sw_err_cnt,
> +                                   h->qs[i]->tx_ring.stats.io_err_cnt);

There is actually a more significant problem with this code, which I
failed to notice when doing the original bugfix: 

You have a sysfs interface here that exports internal data of the
device that should not be visible like this. One problem is that
the io_base is a kernel pointer that must not be visible to non-root
users (so we don't easily create an attack surface for exploits).
Another problem is that the format is not documented in Documentation/ABI/
and that you have multiple values in one sysfs file here.

It would probably be better to completely remove that sysfs interface, and
to use the ethtool netlink interface to export them.

	Arnd
--
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