All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Cen Zhang <zzzccc427@gmail.com>
Cc: Takashi Iwai <tiwai@suse.de>,
	perex@perex.cz, tiwai@suse.com, phucduc.bui@gmail.com,
	kees@kernel.org, blum@kernel.org, hui.wang@canonical.com,
	kai.vehmanen@linux.intel.com, linux-sound@vger.kernel.org,
	linux-kernel@vger.kernel.org, baijiaju1990@gmail.com,
	jjzuming@gmail.com
Subject: Re: [PATCH] ALSA: jack: Protect debugfs setup against control removal
Date: Fri, 16 Oct 2026 15:06:43 +0200	[thread overview]
Message-ID: <87cxt9u74s.wl-tiwai@suse.de> (raw)
In-Reply-To: <CAFRLqsVH=Xkeoh2icgSftWCnk3uEUGPst=N3qeGoZ7Zy9+W6WA@mail.gmail.com>

On Fri, 16 Oct 2026 15:00:37 +0200,
Cen Zhang wrote:
> 
> Hi Takashi,
> 
> Takashi Iwai <tiwai@suse.de> 于2026年10月16日周五 19:13写道:
> >
> > On Fri, 16 Oct 2026 04:04:37 +0200,
> > Cen Zhang wrote:
> > >
> > > The jack control attachment must remain alive while its injection
> > > interface is being initialized. snd_jack_kctl_add() links the attachment
> > > to jack->kctl_list and then calls snd_jack_debugfs_add_inject_node(),
> > > which dereferences jack_kctl->kctl to read the control name. Neither the
> > > list insertion nor the debugfs setup holds controls_rwsem.
> > >
> > > With CONFIG_SND_JACK_INJECTION_DEBUG enabled, an independent kernel
> > > control owner can remove the newly linked control in this order:
> > >
> > >   1. The creator publishes the control with snd_ctl_add(), which releases
> > >      controls_rwsem, and links its attachment in snd_jack_kctl_add().
> > >   2. The remover takes controls_rwsem for writing in snd_ctl_remove().
> > >      __snd_ctl_remove() unlinks the control and calls snd_ctl_free_one().
> > >      snd_jack_kctl_private_free() unlinks and frees the attachment, then
> > >      snd_ctl_free_one() frees the control.
> > >   3. The creator enters snd_jack_debugfs_add_inject_node() and reads the
> > >      control name through the freed attachment.
> > >
> > > This causes a use-after-free read during jack creation. Hold the existing
> > > controls_rwsem for reading before linking the attachment and until the
> > > debugfs helper returns. The remover's write lock then keeps the attachment
> > > and its control alive throughout this post-publication setup interval.
> > >
> > > KASAN report as below:
> > >
> > >     BUG: KASAN: slab-use-after-free in snd_jack_kctl_add+0x50b/0x5e0
> > >     Read of size 8 at addr ffff88810d8def00 by task kworker/2:1/68
> > >
> > >     CPU: 2 UID: 0 PID: 68 Comm: kworker/2:1 Tainted: G           O        7.2.0-rc6-pmb-bt-functional-v1+ #1 PREEMPT(lazy)
> > >     Tainted: [O]=OOT_MODULE
> > >     Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> > >     Workqueue: events azx_probe_work [snd_hda_intel]
> > >     Call Trace:
> > >      <TASK>
> > >      dump_stack_lvl+0x93/0xd0
> > >      print_report+0xce/0x630
> > >      ? snd_jack_kctl_add+0x50b/0x5e0
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? __virt_addr_valid+0x20d/0x410
> > >      ? snd_jack_kctl_add+0x50b/0x5e0
> > >      kasan_report+0xe0/0x110
> > >      ? snd_jack_kctl_add+0x50b/0x5e0
> > >      snd_jack_kctl_add+0x50b/0x5e0
> > >      ? snd_jack_kctl_add+0x5/0x5e0
> > >      snd_jack_new+0x347/0x4c0
> > >      snd_hda_jack_add_kctl_mst+0x19e/0x540 [snd_hda_codec]
> > >      add_jack_kctl+0x1b9/0x2c0 [snd_hda_codec]
> > >      ? __pfx_add_jack_kctl+0x10/0x10 [snd_hda_codec]
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? _raw_spin_unlock_irqrestore+0x44/0x60
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? pmbd_probe_hit_cookie+0xee/0x1c0
> > >      snd_hda_jack_add_kctls+0x212/0x910 [snd_hda_codec]
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      snd_hda_gen_build_controls+0x23a/0x6a0 [snd_hda_codec_generic]
> > >      ? __pfx_snd_hda_gen_build_controls+0x10/0x10 [snd_hda_codec_generic]
> > >      snd_hda_codec_build_controls+0x2e4/0x9f0 [snd_hda_codec]
> > >      ? __pfx_snd_hda_codec_build_controls+0x10/0x10 [snd_hda_codec]
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? snd_hda_codec_build_pcms+0x4d8/0x6e0 [snd_hda_codec]
> > >      hda_codec_driver_probe+0x26e/0x590 [snd_hda_codec]
> > >      ? __pfx_hda_codec_driver_probe+0x10/0x10 [snd_hda_codec]
> > >      really_probe+0x21f/0xa90
> > >      __driver_probe_device+0x1ec/0x490
> > >      driver_probe_device+0x4f/0x130
> > >      __device_attach_driver+0x1d2/0x2e0
> > >      ? __pfx___device_attach_driver+0x10/0x10
> > >      bus_for_each_drv+0x155/0x1d0
> > >      ? __pfx_bus_for_each_drv+0x10/0x10
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? trace_hardirqs_on+0x18/0x160
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? _raw_spin_unlock_irqrestore+0x44/0x60
> > >      __device_attach+0x1c3/0x490
> > >      ? __pfx___device_attach+0x10/0x10
> > >      ? snd_hda_codec_configure+0x4a2/0x690 [snd_hda_codec]
> > >      snd_hda_codec_configure+0x4c5/0x690 [snd_hda_codec]
> > >      ? __pfx_snd_hda_codec_configure+0x10/0x10 [snd_hda_codec]
> > >      ? __pfx_azx_probe_codecs+0x10/0x10 [snd_hda_codec]
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? request_threaded_irq+0x21f/0x380
> > >      azx_codec_configure+0x65/0x200 [snd_hda_codec]
> > >      azx_probe_work+0x4d8/0x23b0 [snd_hda_intel]
> > >      ? __pfx_azx_probe_work+0x10/0x10 [snd_hda_intel]
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? lock_release+0xc8/0x280
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      process_one_work+0x908/0x19c0
> > >      ? __pfx_process_one_work+0x10/0x10
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? lock_is_held_type+0x8f/0x100
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      worker_thread+0x65c/0xe40
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? __kthread_parkme+0x175/0x220
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? __pfx_worker_thread+0x10/0x10
> > >      kthread+0x34f/0x460
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? __pfx_kthread+0x10/0x10
> > >      ret_from_fork+0x659/0x940
> > >      ? __pfx_ret_from_fork+0x10/0x10
> > >      ? srso_alias_return_thunk+0x5/0xfbef5
> > >      ? __switch_to+0x74f/0xf80
> > >      ? __pfx_kthread+0x10/0x10
> > >      ret_from_fork_asm+0x1a/0x30
> > >      </TASK>
> > >
> > >     Allocated by task 68:
> > >      kasan_save_stack+0x33/0x60
> > >      kasan_save_track+0x14/0x30
> > >      __kasan_kmalloc+0xaa/0xb0
> > >      __kmalloc_cache_noprof+0x251/0x610
> > >      snd_jack_kctl_new+0x80/0x180
> > >      snd_jack_new+0x4b/0x4c0
> > >      snd_hda_jack_add_kctl_mst+0x19e/0x540 [snd_hda_codec]
> > >      add_jack_kctl+0x1b9/0x2c0 [snd_hda_codec]
> > >      snd_hda_jack_add_kctls+0x212/0x910 [snd_hda_codec]
> > >      snd_hda_gen_build_controls+0x23a/0x6a0 [snd_hda_codec_generic]
> > >      snd_hda_codec_build_controls+0x2e4/0x9f0 [snd_hda_codec]
> > >      hda_codec_driver_probe+0x26e/0x590 [snd_hda_codec]
> > >      really_probe+0x21f/0xa90
> > >      __driver_probe_device+0x1ec/0x490
> > >      driver_probe_device+0x4f/0x130
> > >      __device_attach_driver+0x1d2/0x2e0
> > >      bus_for_each_drv+0x155/0x1d0
> > >      __device_attach+0x1c3/0x490
> > >      snd_hda_codec_configure+0x4c5/0x690 [snd_hda_codec]
> > >      azx_codec_configure+0x65/0x200 [snd_hda_codec]
> > >      azx_probe_work+0x4d8/0x23b0 [snd_hda_intel]
> > >      process_one_work+0x908/0x19c0
> > >      worker_thread+0x65c/0xe40
> > >      kthread+0x34f/0x460
> > >      ret_from_fork+0x659/0x940
> > >      ret_from_fork_asm+0x1a/0x30
> > >
> > >     Freed by task 436:
> > >      kasan_save_stack+0x33/0x60
> > >      kasan_save_track+0x14/0x30
> > >      kasan_save_free_info+0x3b/0x60
> > >      __kasan_slab_free+0x5f/0x80
> > >      kfree+0x307/0x580
> > >      snd_jack_kctl_private_free+0x154/0x200
> > >      __snd_ctl_remove+0x29f/0x3b0
> > >      snd_ctl_remove+0x46/0x70
> > >      remove_control_set+0x110/0x1b0 [pmbd_alsa_control_remove]
> > >      param_attr_store+0x191/0x2f0
> > >      module_attr_store+0x5d/0x90
> > >      sysfs_kf_write+0xed/0x140
> > >      kernfs_fop_write_iter+0x34b/0x500
> > >      vfs_write+0x637/0x1010
> > >      ksys_write+0x111/0x200
> > >      do_syscall_64+0x115/0x6a0
> > >      entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > >
> > >     The buggy address belongs to the object at ffff88810d8def00
> > >      which belongs to the cache kmalloc-64 of size 64
> > >     The buggy address is located 0 bytes inside of
> > >      freed 64-byte region [ffff88810d8def00, ffff88810d8def40)
> > >
> > >     The buggy address belongs to the physical page:
> > >     page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x10d8de
> > >     flags: 0x200000000000000(node=0|zone=2)
> > >     page_type: f5(slab)
> > >     raw: 0200000000000000 ffff8881000428c0 dead000000000100 dead000000000122
> > >     raw: 0000000000000000 0000000000200020 00000000f5000000 0000000000000000
> > >     page dumped because: kasan: bad access detected
> > >
> > >     Memory state around the buggy address:
> > >      ffff88810d8dee00: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> > >      ffff88810d8dee80: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> > >     >ffff88810d8def00: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> > >                        ^
> > >      ffff88810d8def80: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc
> > >      ffff88810d8df000: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> > >     ==================================================================
> > >
> > > Fixes: 2d670ea2bd53 ("ALSA: jack: implement software jack injection via debugfs")
> > > Assisted-by: LLM
> > > Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> > > ---
> > >
> > > diff --git a/sound/core/jack.c b/sound/core/jack.c
> > > index 0f04018181acb20f6bf85b11a9f6d4c7d3179e5a..dd604ffd8c815a9828f5e4b30a70b7597c86a37f 100644
> > > --- a/sound/core/jack.c
> > > +++ b/sound/core/jack.c
> > > @@ -418,6 +418,7 @@ static void snd_jack_kctl_private_free(struct snd_kcontrol *kctl)
> > >
> > >  static void snd_jack_kctl_add(struct snd_jack *jack, struct snd_jack_kctl *jack_kctl)
> > >  {
> > > +     guard(rwsem_read)(&jack->card->controls_rwsem);
> > >       jack_kctl->jack = jack;
> > >       list_add_tail(&jack_kctl->list, &jack->kctl_list);
> > >       snd_jack_debugfs_add_inject_node(jack, jack_kctl);
> >
> > That looks too ad hoc.  We need to check the real cause at first.
> >
> > Could you elaborate how this situation is triggered?  In the above
> > stack trace, it looks like that the jack ctl was created via hd-audio
> > codec, but how was it removed?
> >
> >
> > thanks,
> >
> > Takashi
> 
> I'm very sorry for wasting your time. Please disregard this patch.
> I'll be much more careful
> in the future.

OK.  Meanwhile, I think we should work on the jack driver code a bit
more regarding the debugfs: namely, the debugfs is exposed already at
the creation of kctls, so they are accessible before other interface
registration via snd_card_register().  The debugfs addition should be
rather moved to the register phase for avoiding the unexpected access
via debugfs, instead.  I'm going to submit a patch later.


thanks,

Takashi

  reply	other threads:[~2026-10-16 13:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16  2:04 [PATCH] ALSA: jack: Protect debugfs setup against control removal Cen Zhang
2026-10-16 11:13 ` Takashi Iwai
2026-10-16 13:00   ` Cen Zhang
2026-10-16 13:06     ` Takashi Iwai [this message]
2026-10-16 13:30       ` Cen Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87cxt9u74s.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=baijiaju1990@gmail.com \
    --cc=blum@kernel.org \
    --cc=hui.wang@canonical.com \
    --cc=jjzuming@gmail.com \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=phucduc.bui@gmail.com \
    --cc=tiwai@suse.com \
    --cc=zzzccc427@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.