netdev - RE: [PATCH v1 net-next 3/6] net: ena: Add validation for completion descriptors consistency

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: <486d3c51e2014d0ebae36b25be276bf9@amazon.com>
Date: Wed, 8 May 2024 06:01:33 +0000
From: "Arinzon, David" <darinzon@...zon.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: David Miller <davem@...emloft.net>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "Woodhouse, David" <dwmw@...zon.co.uk>, "Machulsky,
 Zorik" <zorik@...zon.com>, "Matushevsky, Alexander" <matua@...zon.com>,
	"Bshara, Saeed" <saeedb@...zon.com>, "Wilson, Matt" <msw@...zon.com>,
	"Liguori, Anthony" <aliguori@...zon.com>, "Bshara, Nafea" <nafea@...zon.com>,
	"Belgazal, Netanel" <netanel@...zon.com>, "Saidi, Ali" <alisaidi@...zon.com>,
	"Herrenschmidt, Benjamin" <benh@...zon.com>, "Kiyanovski, Arthur"
	<akiyano@...zon.com>, "Dagan, Noam" <ndagan@...zon.com>, "Agroskin, Shay"
	<shayagr@...zon.com>, "Itzko, Shahar" <itzko@...zon.com>, "Abboud, Osama"
	<osamaabb@...zon.com>, "Ostrovsky, Evgeny" <evostrov@...zon.com>, "Tabachnik,
 Ofir" <ofirt@...zon.com>
Subject: RE: [PATCH v1 net-next 3/6] net: ena: Add validation for completion
 descriptors consistency

> > +             if (unlikely((status & ENA_ETH_IO_RX_CDESC_BASE_FIRST_MASK)
> >>
> > +                 ENA_ETH_IO_RX_CDESC_BASE_FIRST_SHIFT && count != 0)) {
> > +                     struct ena_com_dev *dev =
> > + ena_com_io_cq_to_ena_dev(io_cq);
> > +
> > +                     netdev_err(dev->net_device,
> > +                                "First bit is on in descriptor #%d on q_id: %d, req_id:
> %u\n",
> > +                                count, io_cq->qid, cdesc->req_id);
> > +                     return -EFAULT;
> 
> This is really asking to be a devlink health reporter.
> You can dump the information to the user and get the event counter for
> free.
> 
> But if you don't want to use that - please at least rate limit the message.

Hi Jakub,

Thank you for reviewing the patch.

We have an action item to review the devlink health mechanism to see how it can benefit
our solution. Thank you for raising this point.

Regarding this print, I don't expect it to be printed more than once,
as a failure here returns -EFAULT which triggers an error flow and an ENA reset sequence,
which will also stop the TX and RX queues as part of it, and no further descriptors will
be processed until the reset sequence is complete.

Please let us know if you still have concerns regarding this print.