On Apr 15, 2021, at 4:55 AM, Takashi Iwai <tiwai@suse.demailto:tiwai@suse.de> wrote:

On Thu, 15 Apr 2021 03:47:14 +0200, Gong, Sishuai wrote:

Hi,

We found a data race in sound/core/control.c in linux-5.12-rc3 and we are able to reproduce it under x86. In general, we found when 2 kernel threads are both running snd_ctl_elem_add(), one may read a stale value of card->user_ctl_count, as shown below.

Currently, we haven’t found any explicit errors due to this data race, but it looks the reader threads may operate in an inconsistent state, where card->user_ctl_count + 1 is actually bigger than MAX_USER_CONTROLS, so we want to point it out.

Thread 1 Thread 2 //snd_ctl_elem_add() //snd_ctl_elem_add() if (card->user_ctl_count + 1 > MAX_USER_CONTROLS) return -ENOMEM;

card->user_ctl_count++; unlock: up_write(&card->controls_rwsem); return err;

Thanks for the report. Indeed this is a race at read. Thanks for your reply. I am glad I could be helpful. OTOH, it's not much serious since this check is only for a sort of soft-limit to avoid the memory exhaustion. If any, we can add the same card->user_ctl_count check just before __snd_ctl_add_replace() call, but maybe not worth for extra work.

It looks like this is still a harmful data race, although not much serious, as you said. Possibly, there could be a crazy number of snd_ctl_elem_add() passing this check but pausing before increasing card->user_ctl_count (so other snd_ctl_elem_add() can still pass the check). Once they all finish, they can cause a crazy card->user_ctl_count value and also memory exhaustion.

Anyway, since the code will be changed so probably it is not worth of extra fix. To be noted, the relevant code was already changed significantly for 5.13 (currently in for-next branch), hence the fix I mentioned in the above won't be applicable. And, I noticed that a similar problem is seen there, even worse -- a race may happen at the write side in this case, which needs a fix. Will take a deeper look later.

thanks,

Takashi