netdev - xen-netback hotplug-status regression bug

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-next>] [day] [month] [year] [list]

Date:   Sat, 10 Apr 2021 19:25:03 +0100
From:   Michael Brown <mcb30@...e.org>
To:     Wei Liu <wei.liu@...nel.org>, Paul Durrant <paul@....org>,
        xen-devel@...ts.xenproject.org, netdev@...r.kernel.org,
        Paul Durrant <pdurrant@...zon.com>
Subject: xen-netback hotplug-status regression bug

Commit https://github.com/torvalds/linux/commit/1f25657 ("xen-netback: 
remove 'hotplug-status' once it has served its purpose") seems to have 
introduced a regression that prevents a vif frontend from transitioning 
more than once into Connected state.

As far as I can tell:

- The defined vif script (e.g. /etc/xen/scripts/vif-bridge) executes 
only once, at domU startup, and sets 
backend/vif/<domU>/0/hotplug-status="connected"

- When the frontend first enters Connected state, 
drivers/net/xen-netback/xenbus.c's connect() sets up a watch on 
"hotplug-status" with the callback function hotplug_status_changed()

- When hotplug_status_changed() is triggered by the watch, it 
transitions the backend to Connected state and calls xenbus_rm() to 
delete the "hotplug-status" attribute.

If the frontend subsequently disconnects and reconnects (e.g. 
transitions through Closed->Initialising->Connected) then:

- Nothing recreates "hotplug-status"

- When the frontend re-enters Connected state, connect() sets up a watch 
on "hotplug-status" again

- The callback hotplug_status_changed() is never triggered, and so the 
backend device never transitions to Connected state.

Reverting the commit would fix this bug, but would obviously also 
reintroduce the race condition that the commit was designed to avoid.

I'm happy to put together a patch, if one of the maintainers could 
suggest a sensible design approach.

I'm not a list member, so please CC me directly on replies.

Thanks,

Michael