Re: [PATCH] ALSA: info: Serialize freeing with recursive registration

[Cen Zhang]

Hi Takashi,

Takashi Iwai <tiwai@xxxxxxx> 于2026年10月17日周六 18:47写道：
>
> I meant something like below (totally untested)
>
>
> Takashi
>
> -- 8< --
> diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
> index f1fa42e634fc..2e686bd6c5f9 100644
> --- a/sound/core/oss/mixer_oss.c
> +++ b/sound/core/oss/mixer_oss.c
> @@ -1309,13 +1309,18 @@ static int snd_mixer_oss_free1(void *private)
>  static int snd_mixer_oss_notify_handler(struct snd_card *card, int cmd)
>  {
>         struct snd_mixer_oss *mixer;
> +       int err;
>
>         if (cmd == SND_MIXER_OSS_NOTIFY_REGISTER) {
> -               int idx, err;
> +               int idx;
>
>                 mixer = kzalloc_objs(*mixer, 2);
>                 if (mixer == NULL)
>                         return -ENOMEM;
> +               if (!try_module_get(THIS_MODULE)) {
> +                       kfree(mixer);
> +                       return -ENODEV;
> +               }
>                 mutex_init(&mixer->reg_mutex);
>                 err = snd_register_oss_device(SNDRV_OSS_DEVICE_TYPE_MIXER,
>                                               card, 0,
> @@ -1325,6 +1330,7 @@ static int snd_mixer_oss_notify_handler(struct snd_card *card, int cmd)
>                                 "unable to register OSS mixer device %i:%i\n",
>                                 card->number, 0);
>                         kfree(mixer);
> +                       module_put(THIS_MODULE);
>                         return err;
>                 }
>                 mixer->oss_dev_alloc = 1;
> @@ -1358,7 +1364,9 @@ static int snd_mixer_oss_notify_handler(struct snd_card *card, int cmd)
>                 if (cmd == SND_MIXER_OSS_NOTIFY_DISCONNECT)
>                         return 0;
>                 snd_mixer_oss_proc_done(mixer);
> -               return snd_mixer_oss_free1(mixer);
> +               err = snd_mixer_oss_free1(mixer);
> +               module_put(THIS_MODULE);
> +               return err;
>         }
>         return 0;
>  }
> diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
> index bd19328dca9e..3d35f9d2d675 100644
> --- a/sound/core/oss/pcm_oss.c
> +++ b/sound/core/oss/pcm_oss.c
> @@ -3151,6 +3151,9 @@ static void register_oss_dsp(struct snd_pcm *pcm, int index)
>
>  static int snd_pcm_oss_register_minor(struct snd_pcm *pcm)
>  {
> +       if (!try_module_get(THIS_MODULE))
> +               return -ENODEV;
> +
>         pcm->oss.reg = 0;
>         if (dsp_map[pcm->card->number] == (int)pcm->device) {
>                 char name[128];
> @@ -3207,6 +3210,7 @@ static int snd_pcm_oss_unregister_minor(struct snd_pcm *pcm)
>  {
>         snd_pcm_oss_disconnect_minor(pcm);
>         snd_pcm_oss_proc_done(pcm);
> +       module_put(THIS_MODULE);
>         return 0;
>  }
>

Thank you for the review and suggestions. This looks good to me.

While reviewing the draft, I noticed one detail we may need to account
for: snd_pcm_free() calls
n_unregister even for a PCM that never reached n_register, for example
during creation failure or
BUILD-state cleanup. We may therefore need per-PCM bookkeeping so that
unregister drops a
module reference only if one was successfully acquired.

I would also like to check the last-reference drop against callback
completion and notifier traversal,
to make sure module teardown cannot overtake either.

I will send a patch soon.

Thanks again.

Best regards,
Cen Zhang