Change Details

Suppose the following scenario: 1. Thread A locks CHN, sets the trigger to ABORT, and unlocks CHN. It then locks PCM and _removes_ CHN from the list. 2. In the meantime, since thread A unlocked CHN, thread B has locked it, set the trigger to START, unlocked it, and is now blocking on PCM held by thread A. 3. At the same time, thread C locks CHN, sets the trigger back to ABORT, unlocks CHN, and is also blocking on PCM. However, when thread A unlocks PCM, because thread C is higher-priority than thread B, it picks up the PCM lock instead of thread B, and because CHN is already removed from the list, and thread B hasn't added it back yet, we take a page fault in CHN_REMOVE() by trying to remove a non-existent element. To fix this, have PCM locked during both the trigger setting, as well as adding/removing a channel to/from the list, so that we have a single section instead of two. The same scenario applies to vchan_trigger(), where the parent channel's lock acts as the PCM one. Sponsored by: The FreeBSD Foundation MFC after: 2 days

Consider the following scenario: 1. CHN currently has its trigger set to PCMTRIG_STOP. 2. Thread A locks CHN, calls CHANNEL_TRIGGER(PCMTRIG_START), sets the trigger to PCMTRIG_START and unlocks. 3. Thread B picks up the lock, calls CHANNEL_TRIGGER(PCMTRIG_ABORT) and returns a non-zero value, so it returns from chn_trigger() as well. 4. Thread A picks up the lock and adds CHN to the list, which is _wrong_, because the last call to CHANNEL_TRIGGER() was with PCMTRIG_ABORT, meaning the channel is stopped, yet we are adding it to the list and marking it as started. Another problematic scenario: 1. Thread A locks CHN, sets the trigger to PCMTRIG_ABORT, and unlocks CHN. It then locks PCM and _removes_ CHN from the list. 2. In the meantime, since thread A unlocked CHN, thread B has locked it, set the trigger to PCMTRIG_START, unlocked it, and is now blocking on PCM held by thread A. 3. At the same time, thread C locks CHN, sets the trigger back to PCMTRIG_ABORT, unlocks CHN, and is also blocking on PCM. However, once thread A unlocks PCM, because thread C is higher-priority than thread B, it picks up the PCM lock instead of thread B, and because CHN is already removed from the list, and thread B hasn't added it back yet, we take a page fault in CHN_REMOVE() by trying to remove a non-existent element. To fix the former scenario, set the channel trigger before the call to CHANNEL_TRIGGER() (could also come after, doesn't really matter) and check if anything changed one we lock CHN back. To fix the latter scenario, use the SAFE variants of CHN_INSERT_HEAD() and CHN_REMOVE(). A similar scenario can occur in vchan_trigger(), so do the trigger setting after we've locked the parent channel. Sponsored by: The FreeBSD Foundation MFC after: 2 days

SupposeConsider the following scenario: 1. Thread A locks CHN, sets theCHN currently has its trigger to ABORT, and unlocks CHN. Itset to PCMTRIG_STOP. 2. Thread A locks CHN, calls CHANNEL_TRIGGER(PCMTRIG_START), sets the then lockstrigger to PCM and _removes_ CHN from the listTRIG_START and unlocks. 3. Thread B picks up the lock, calls CHANNEL_TRIGGER(PCMTRIG_ABORT) and returns a non-zero value, so it returns from chn_trigger() as well. 4. Thread A picks up the lock and adds CHN to the list, which is _wrong_, because the last call to CHANNEL_TRIGGER() was with PCMTRIG_ABORT, meaning the channel is stopped, yet we are adding it to the list and marking it as started. Another problematic scenario: 1. Thread A locks CHN, sets the trigger to PCMTRIG_ABORT, and unlocks CHN. It then locks PCM and _removes_ CHN from the list. 2. In the meantime, since thread A unlocked CHN, thread B has locked it, set the trigger to PCMTRIG_START, unlocked it, and is now blocking on PCM PCM held by thread A. 3. At the same time, thread C locks CHN, sets the trigger back to ABORT, unlocks CHNPCMTRIG_ABORT, and is also bunlocking on PCM.s CHN, However,and is also blocking on PCM. when thread AHowever, unlocks PCM, becausonce thread C is higher-priority than thread BA unlocks PCM, itbecause thread C is higher-priority than thread B, it picks up the PCM lock instead of thread B, and because CHN is already removed from the list, and thCHN is alread B hasn't added it back yety removed from the list, we takeand thread B hasn't added it back yet, we take a page fault in CHN_REMOVE() by trying to remove a non-existent non-existent element. To fix this, have PCM locked during both the trigger settinge former scenario, as well asset the channel trigger before the call to adding/removing a channel to/from the listCHANNEL_TRIGGER() (could also come after, so that we have a singledoesn't really matter) and section instead of twocheck if anything changed one we lock CHN back. The same scenario applies to vchan_trigger()To fix the latter scenario, where the parent channel'suse the SAFE variants of CHN_INSERT_HEAD() lock acts as the PCM oneand CHN_REMOVE(). A similar scenario can occur in vchan_trigger(), so do the trigger setting after we've locked the parent channel. Sponsored by: The FreeBSD Foundation MFC after: 2 days