linux-kernel - Re: [External] Re: [PATCH 1/1] Revert "drivers/net/ftgmac100: fix DHCP potential failure with systemd"

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: <CA+J-oUvsnovtMGKVAWnw-eG6SZvNZLEOsf-8zp6BEwzq4_wvhw@mail.gmail.com>
Date: Mon, 15 Sep 2025 19:57:05 +0800
From: Zhang Jian <zhangjian.3032@...edance.com>
To: Andrew Lunn <andrew@...n.ch>, Jacky Chou <jacky_chou@...eedtech.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, andrew+netdev@...n.ch, 
	guoheyi@...ux.alibaba.com, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Heiner Kallweit <hkallweit1@...il.com>, Uwe Kleine-König <u.kleine-koenig@...libre.com>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH 1/1] Revert "drivers/net/ftgmac100: fix
 DHCP potential failure with systemd"

On Sat, Sep 13, 2025 at 4:25 AM Andrew Lunn <andrew@...n.ch> wrote:
>
> On Fri, Sep 12, 2025 at 11:45:38AM +0800, Jian Zhang wrote:
> > This reverts commit 1baf2e50e48f10f0ea07d53e13381fd0da1546d2.
>
> > * rtnetlink is setting the link down
> > * the PHY state_queue is triggered and calls ftgmac100_adjust_link
> > -     /* Release phy lock to allow ftgmac100_reset to acquire it, keeping lock
> > -      * order consistent to prevent dead lock.
> > -      */
> > -     if (netdev->phydev)
> > -             mutex_unlock(&netdev->phydev->lock);
> > -
> > -     ftgmac100_reset(priv);
> > -
> > -     if (netdev->phydev)
> > -             mutex_lock(&netdev->phydev->lock);
> > -
> > +     /* Reset the adapter asynchronously */
> > +     schedule_work(&priv->reset_task);
> >  }
>
> So we are swapping one set of bugs for another set of bugs.
>
Yes, If the full reset is necessary.
In commit 855944ce1cc4 (“ftgmac100: Add a reset task and use it for
link changes”, about 8 years ago)
it was mentioned that a full hardware reset is required, and it also
notes that the rtnl lock is held.

> No other adjust_link callback messes with locks like this.  Have you
> investigated what actually needs to be done by adjust_link?
>
> Determining maccr in ftgmac100_reset_and_config_mac() look relevant.
> Does it actually need a reset, or is it sufficient to just set the
> bits in FTGMAC100_OFFSET_MACCR?
>

I checked most drivers’ adjust_link callbacks, and they basically only
write to a few MAC controller registers.
I also tried calling only `ftgmac100_reset_and_config_mac` and did
some tests, the network works fine
but I’m not sure this change is correct, which is why I wanted to
revert to the previous code.

> ftgmac100_config_pause() is called from ftgmac100_set_pauseparam()
> suggesting it can be called at any time.
>
> So i think the crux of the problem is what needs to happen to set bits
> in MACCR.
>
Yes, I think so too. However, here we just need to do some necessary
register configurations,
and that should be sufficient.

Hi Jacky;

I’m not sure if you’re familiar with these, Could you give us some suggestions?

>         Andrew
Jian