netdev - Re: [PATCH net] net: enetc: avoid deadlock in enetc_tx_onestep

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: <CAKgT0UdJRkds8FFGHppLWOQoAEgjRw6SjccaxPGBg4d7OWpaqg@mail.gmail.com>
Date:   Thu, 12 Jan 2023 13:54:57 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        "Y . b . Lu" <yangbo.lu@....com>
Subject: Re: [PATCH net] net: enetc: avoid deadlock in enetc_tx_onestep_tstamp()

On Thu, Jan 12, 2023 at 1:36 PM Vladimir Oltean <vladimir.oltean@....com> wrote:
>
> On Thu, Jan 12, 2023 at 01:29:21PM -0800, Alexander Duyck wrote:
> > One other question I had. How do you handle the event that
> > enetc_start_xmit returns NETDEV_TX_BUSY or causes the packet to go
> > down the drop_packet_err path?
>
> We don't. If the enetc_start_xmit() asks the qdisc to requeue the skb
> via NETDEV_TX_BUSY, we aren't going to do that, because we aren't the
> qdisc, or if the packet just gets dropped without being mapped into the
> TX ring, ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS will remain set with no
> possibility of ever becoming unset ever again.

That is a separate issue then right? Just wanted to confirm I wasn't
missing something. I am assuming that leaving it set forever would be
a bad thing.

If NETDEV_TX_BUSY is triggered it is a memory leak if it is hit from
the enetc_tx_onestep_tstamp function correct?

Also what mechanism do you have in place to clean out the tx_skb queue
and clear the flag if the ring is stopped due to something such as
enetc_close being called? It seems like this is missing some logic to
handle the event that somebody were to do a ip link set <iface>
down/up as the stale packets would be left in the ring unless I am
missing something else.