Race condition

[illiliti]

Due to lack of synchronization, a race condition may occur when e.g other programs/services need rw access to /dev/input/*. Since mdevd doesn't provide a way to request its state of processing coldplug uevents, those programs/services have no synchronization with mdevd which may lead to breakage due to bad permissions in /dev/input/*.

This is tricky issue to fix i think. Udev has udevadm settle which blocks until all events are processed. Mdev has mdev -s which set proper permissions in /dev/* without need for daemon. What mdevd should do? Looking forward to your ideas!

[skarnet]

Well, that's in part what the -O option - that you asked me for - does: the uevent only gets rebroadcast when mdevd is done handling it. So with -O 4, if you receive a /dev/input uevent on netlink group 4, it means that anything related to /dev/input in /etc/mdev.conf has already been processed. Is there another point of synchronization that you need? Or is it only a question of interfaces?

[illiliti]

-O fixes race condition for hotplugged devices and not really related to this topic.

Consider the following example:

#!/bin/sh
# this is /sbin/init

# some initialization is going on here

mdevd & # this doesn't block
mdevd-coldplug # trigger uevents for cold-plugged devices
# synchronization is needed here
sway # this will fail because mdevd doesn't have enough time to handle uevents and set permissions for devnodes

Sway(well libinput) uses /dev/input to initialize cold-plugged devices. If it encounter devnode with bad permissions(because of race condition), it will exit(1). In order to fix this, some kind of synchronization(like udevadm settle) is needed between mdevd-coldplug and sway to ensure that all uevents are processed before running sway.

[skarnet]

Technically, sway doesn't need all uevents to be processed; it just needs the /dev/input processing to be done. So you could get by with -O. 😛

But yes, I understand that what you want is an equivalent to udevadm settle, for simplicity's sake. It is pretty difficult to implement, but would it work for you if mdevd-coldplug had an option that made it synchronous, i.e. it would only exit when the last coldplug uevent has been processed? I may have an idea for this.

[illiliti]

Technically, sway doesn't need all uevents to be processed; it just needs the /dev/input processing to be done. So you could get by with -O. stuck_out_tongue

Sway is just an example. There are also potential problems with kernel modules and modprobe due to this race condition.

would it work for you if mdevd-coldplug had an option that made it synchronous, i.e. it would only exit when the last coldplug uevent has been processed? I may have an idea for this.

Maybe. Depends on how you want to implement it.

[skarnet]

The idea would be to make mdevd-coldplug listen to the netlink group mdevd is rebroadcasting too, and only exit when it gets an uevent for the last device it triggered.

[illiliti]

This is interesting. I wonder how you intend to determine that last device.

[illiliti]

I wonder how you intend to determine that last device.

I found something interesting: https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-uevent

Turns out you can pass your own variables to uevent. This could be leveraged to implement detection of the last device.

[skarnet]

Oh, I don't think you need this. You just need to record the DEVPATH of the last device (which can be trivially achieved by keeping the DEVPATH of every device in a variable that gets overwritten every loop iteration, and the last value when the loop exits is the correct one).

[illiliti]

Good idea

[skarnet]

@illiliti Can you test with the latest git head? mdevd-coldplug -O4 should block until the last event has been processed, if you're running mdevd -O4 as well.

[illiliti]

I got segfault at

mdevd/src/mdevd/mdevd-coldplug.c

Line 155 in 6e21f54

if (!strcmp(x, mdev)) break ;

gdb says x is 0x0

[skarnet]

That's worrying: is there a path where the MDEV variable is not set by mdevd? Can I see your mdev.conf file? Thanks for the report, I'll have a look.

[illiliti]

is there a path where the MDEV variable is not set by mdevd?

/devices/system/memory/memory32. That's where it segfaults. It has DEVPATH but not MDEV it seems

Can I see your mdev.conf file?

https://github.com/kiss-community/repo/blob/master/extra/mdevd/files/mdevd.conf

Plus

$MODALIAS=.* root:root 660 @modprobe -q "$MODALIAS"

[skarnet]

Yeah that makes sense. The MDEV variable is actually only supposed to be exported in spawned helper commands, it doesn't belong to the uevent itself, and the fact that it was visible in the rebroadcasted events was a bug. My own mdev.conf was hiding the issue.

I removed MDEV from the rebroadcasted events and made mdevd-coldplug test against the last part of DEVPATH instead. Can you please test this version?

[illiliti]

I don't see segfault anymore, but race condition is still present.