4.13 regression: get_kctl_0dB_offset doesn't handle all possible callbacks

4.13 regression: get_kctl_0dB_offset doesn't handle all possible callbacks

[PaX Team]

hello devs,

upstream commit 99b5c5bb9a5435a5ae5d46445ac0f2bf6aa5ee52 removed the use
of set_fs in get_kctl_0dB_offset under the assumption that the only runtime
value of kctl->tlv.c was snd_hda_mixer_amp_tlv. alas, recently, the KERNEXEC
and UDEREF features in PaX reported a violation of this assumption as it
turns out that _snd_ctl_add_slave sets this callback to slave_tlv_cmd instead.

it seems to me that there're several other possible values for this callback
so it's not clear why it was assumed that get_kctl_0dB_offset would only be
called with only one of them in practice, but nevertheless, it happens here
so clearly something is wrong. the backtrace that shows how things go wrong
in this case:

 PAX: suspicious general protection fault: 0000 [#1] PREEMPT SMP
 Modules linked in:
 CPU: 4 PID: 31 Comm: kworker/4:0 Not tainted 4.13.5-i386-pax #17
 Workqueue: events azx_probe_work
 task: eb61c880 task.stack: eb62a000
 EIP: 0060:[<009bbd2d>] init_slave_0dB+0x2d/0xa0
 EFLAGS: 00210202 CPU: 4
 EAX: 00991fd0 EBX: e9883a80 ECX: e9883a80 EDX: eb62bd74
 ESI: eb62bd74 EDI: e9098800 EBP: eb62bd08 ESP: eb62bcec
  DS: 0068 ES: 0068 FS: 00d8 GS: 0068 SS: 0068
 CR0: 80050033 CR2: 00000000 CR3: 03b2d100 CR4: 000406f0 shadow CR4: 000406f0
 Call Trace:
  [<009bb469>] map_slaves+0xb9/0xe0
  [<009bce0e>] __snd_hda_add_vmaster+0xde/0x110
  [<009c82f0>] snd_hda_gen_build_controls+0x1a0/0x1b0
  [<009d0a4d>] cx_auto_build_controls+0xd/0x70
  [<009bdf76>] snd_hda_codec_build_controls+0x186/0x1d0
  [<009b92ad>] hda_codec_driver_probe+0x6d/0xf0
  [<006a3be9>] driver_probe_device+0x289/0x420
  [<006a3ed6>] __device_attach_driver+0x76/0x100
  [<006a1edf>] bus_for_each_drv+0x3f/0x70
  [<006a3813>] __device_attach+0xa3/0x110
  [<006a3f9d>] device_initial_probe+0xd/0x10
  [<006a2d6f>] bus_probe_device+0x6f/0x80
  [<006a100f>] device_add+0x2cf/0x590
  [<009d8d8c>] snd_hdac_device_register+0xc/0x40
  [<009b90b4>] snd_hda_codec_configure+0x34/0x140
  [<009c2875>] azx_codec_configure+0x25/0x50
  [<009d8081>] azx_probe_continue+0x621/0x9e0
  [<009d84bd>] azx_probe_work+0xd/0x10
  [<0006fff2>] process_one_work+0x122/0x2a0
  [<000701a9>] worker_thread+0x39/0x390
  [<00074df2>] kthread+0xe2/0x110
  [<00cee619>] ret_from_fork+0x19/0x30
 Code: e5 57 89 c7 56 89 d6 53 89 cb 83 ec 10 8b 41 6c a9 00 00 00 10 74 09 81 79 58 90 bc 9b 00 74 4e a8 10 74 38 8b 43 58 85 c0 74 31 <83> 38 01 75 2c 8b 48 0c 81 e1 ff ff fe ff 74 21 8b 16 39 d1 
74
 EIP: [<009bbd2d>] init_slave_0dB+0x2d/0xa0 SS:ESP: 0068:eb62bcec

what KERNEXEC on i386 does is that it executes kernel code in its own 0-based
code segment hence the 'low' code addresses. due to the current logic that
checks for SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK in get_kctl_0dB_offset, this
callback address is instead treated as a data pointer (as apparently
SNDRV_CTL_ELEM_ACCESS_TLV_READ is also set) and since it's not a valid kernel
address, it causes a GPF under the UDEREF PaX feature (without UDEREF it'd
have been an oops since such low addresses are not mapped for kernel threads).

on vanilla kernels all this is a silent read of kernel code bytes that are
then interpreted as the tlv[] array content, which is probably not what you
want either.

as for fixing this, first the above mentioned assumption should be re-evaluated.
if it's considered correct then there is some logic bug in my case (i can help
debug it if you tell me what to do) otherwise the current pattern of

   if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK && callback == snd_hda_mixer_amp_tlv) {
     call wrapped function underneath snd_hda_mixer_amp_tlv
   } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
     treat unrecognized callback address as data ptr
   }

should be changed to

   if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK( {
     if (callback == snd_hda_mixer_amp_tlv) {
       call wrapped function underneath snd_hda_mixer_amp_tlv
     } else if (callback == others) {
       handle others, WARN/BUG/etc
     }
   } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
     no longer treat unrecognized callback address as data ptr
   }

and all other callbacks with userland access should be refactored the same
way as snd_hda_mixer_amp_tlv was. i'd also suggest that you at least do the
above suggested if/else pattern change in order to prevent the misuse of
unexpected callbacks in the future.

cheers,
  PaX Team

Re: 4.13 regression: get_kctl_0dB_offset doesn't handle all possible callbacks

[Takashi Sakamoto]

Hi,

On Oct 14 2017 07:46, PaX Team wrote:
> what KERNEXEC on i386 does is that it executes kernel code in its own 0-based
> code segment hence the 'low' code addresses. due to the current logic that
> checks for SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK in get_kctl_0dB_offset, this
> callback address is instead treated as a data pointer (as apparently
> SNDRV_CTL_ELEM_ACCESS_TLV_READ is also set) and since it's not a valid kernel
> address, it causes a GPF under the UDEREF PaX feature (without UDEREF it'd
> have been an oops since such low addresses are not mapped for kernel threads).

There're two ways to use 'struct snd_kcontrol.tlv'; constant array (=.p) an
a handler (= .c). An 'access' flag in each member of
'struct snd_kcontrol.vd' represent which way is used for the member.
Therefore, as long as checking the flag, the 'get_kctl_0dB_offset()' doesn't
handle a pointer to the handler as a pointer to array of TLV container.

> on vanilla kernels all this is a silent read of kernel code bytes that are
> then interpreted as the tlv[] array content, which is probably not what you
> want either.

Yes. Current code include a bug of inappropriate condition statement for the
above. As a result, it allows to handle a pointer to the handler as a
pointer to TLV data. 

> as for fixing this, first the above mentioned assumption should be re-evaluated.
> if it's considered correct then there is some logic bug in my case (i can help
> debug it if you tell me what to do) otherwise the current pattern of
> 
>     if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK && callback == snd_hda_mixer_amp_tlv) {
>       call wrapped function underneath snd_hda_mixer_amp_tlv
>     } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
>       treat unrecognized callback address as data ptr
>     }
> 
> should be changed to
> 
>     if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK( {
>       if (callback == snd_hda_mixer_amp_tlv) {
>         call wrapped function underneath snd_hda_mixer_amp_tlv
>       } else if (callback == others) {
>         handle others, WARN/BUG/etc
>       }
>     } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
>       no longer treat unrecognized callback address as data ptr
>     }

Good enough as a solution. Please test a patch in the end of this message.

> and all other callbacks with userland access should be refactored the same
> way as snd_hda_mixer_amp_tlv was. i'd also suggest that you at least do the
> above suggested if/else pattern change in order to prevent the misuse of
> unexpected callbacks in the future.

This suggestion is better for safety. Do you have some ways to detect the
pattern on current vanilla kernel? Or we should find it by eye-grep?


Thanks

Takashi Sakamoto

======== 8< --------

>From 85896b50aa22bf2f2b5e45456daa16d386602edc Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Sat, 14 Oct 2017 14:08:51 +0900
Subject: [PATCH] ALSA: hda-intel: Fix to handle a pointer to TLV handler
 as a pointer to TLV data

In a design of ALSA control core, an 'access' flag on each entry of
'struct snd_kcontrol.vd' represents the type of 'struct snd_kcontrol.tlv'
for the corresponding element; a pointer to TLV data
(!=SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) or a pointer to handler
(==SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK).

In current implementation of 'get_kctl_0dB_offset()', condition statement
is not proper to distinguish these two cases. As a result, it handles
a pointer to the handler as a pointer to TLV data for some control element
sets. This bug brings invalid references to kernel space. A reporter shows
a sample of backtrace.

PAX: suspicious general protection fault: 0000 [#1] PREEMPT SMP
Modules linked in:
CPU: 4 PID: 31 Comm: kworker/4:0 Not tainted 4.13.5-i386-pax #17
Workqueue: events azx_probe_work
task: eb61c880 task.stack: eb62a000
EIP: 0060:[<009bbd2d>] init_slave_0dB+0x2d/0xa0
EFLAGS: 00210202 CPU: 4
EAX: 00991fd0 EBX: e9883a80 ECX: e9883a80 EDX: eb62bd74
ESI: eb62bd74 EDI: e9098800 EBP: eb62bd08 ESP: eb62bcec
 DS: 0068 ES: 0068 FS: 00d8 GS: 0068 SS: 0068
CR0: 80050033 CR2: 00000000 CR3: 03b2d100 CR4: 000406f0 shadow CR4: 000406f0
Call Trace:
 [<009bb469>] map_slaves+0xb9/0xe0
 [<009bce0e>] __snd_hda_add_vmaster+0xde/0x110
 [<009c82f0>] snd_hda_gen_build_controls+0x1a0/0x1b0
 [<009d0a4d>] cx_auto_build_controls+0xd/0x70
 [<009bdf76>] snd_hda_codec_build_controls+0x186/0x1d0
 [<009b92ad>] hda_codec_driver_probe+0x6d/0xf0
 [<006a3be9>] driver_probe_device+0x289/0x420
 [<006a3ed6>] __device_attach_driver+0x76/0x100
 [<006a1edf>] bus_for_each_drv+0x3f/0x70
 [<006a3813>] __device_attach+0xa3/0x110
 [<006a3f9d>] device_initial_probe+0xd/0x10
 [<006a2d6f>] bus_probe_device+0x6f/0x80
 [<006a100f>] device_add+0x2cf/0x590
 [<009d8d8c>] snd_hdac_device_register+0xc/0x40
 [<009b90b4>] snd_hda_codec_configure+0x34/0x140
 [<009c2875>] azx_codec_configure+0x25/0x50
 [<009d8081>] azx_probe_continue+0x621/0x9e0
 [<009d84bd>] azx_probe_work+0xd/0x10
 [<0006fff2>] process_one_work+0x122/0x2a0
 [<000701a9>] worker_thread+0x39/0x390
 [<00074df2>] kthread+0xe2/0x110
 [<00cee619>] ret_from_fork+0x19/0x30
Code: e5 57 89 c7 56 89 d6 53 89 cb 83 ec 10 8b 41 6c a9 00 00 00 10 74 09 81 79 58 90 bc 9b 00 74 4e a8 10 74 38 8b 43 58 85 c0 74 31 <83> 38 01 75 2c 8b 48 0c 81 e1 ff ff fe ff 74 21 8b 16 39 d1
4
EIP: [<009bbd2d>] init_slave_0dB+0x2d/0xa0 SS:ESP: 0068:eb62bcec

This commit fixes the bug.

Reported-by: PaX Team <pageexec@freemail.hu>
Fixes: 99b5c5bb9a54 ('ALSA: hda - Remove the use of set_fs()')
Cc: <stable@vger.kernel.org> # 4.13+
---
 sound/pci/hda/hda_codec.c | 47 +++++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 3db26c451837..e32a59c42577 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -1809,28 +1809,39 @@ static int get_kctl_0dB_offset(struct hda_codec *codec,
 {
 	int _tlv[4];
 	const int *tlv = NULL;
-	int val = -1;
+	int step;
 
-	if ((kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) &&
-	    kctl->tlv.c == snd_hda_mixer_amp_tlv) {
-		get_ctl_amp_tlv(kctl, _tlv);
-		tlv = _tlv;
-	} else if (kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_READ)
-		tlv = kctl->tlv.p;
-	if (tlv && tlv[0] == SNDRV_CTL_TLVT_DB_SCALE) {
-		int step = tlv[3];
-		step &= ~TLV_DB_SCALE_MUTE;
-		if (!step)
+	if (kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
+		/* TLV data for DB_SCALE is available for amp element only. */
+		if (kctl->tlv.c != snd_hda_mixer_amp_tlv)
 			return -1;
-		if (*step_to_check && *step_to_check != step) {
-			codec_err(codec, "Mismatching dB step for vmaster slave (%d!=%d)\n",
--				   *step_to_check, step);
+
+		get_ctl_amp_tlv(kctl, _tlv);
+		tlv = (const int *)_tlv;
+	} else {
+		if (!(kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_READ))
 			return -1;
-		}
-		*step_to_check = step;
-		val = -tlv[2] / step;
+
+		tlv = kctl->tlv.p;
 	}
-	return val;
+
+	if (tlv[0] != SNDRV_CTL_TLVT_DB_SCALE)
+		return -1;
+
+	step = tlv[3];
+	step &= ~SNDRV_CTL_TLVD_DB_SCALE_MUTE;
+	if (!step)
+		return -1;
+
+	if (*step_to_check && *step_to_check != step) {
+		codec_err(codec,
+			  "Mismatching dB step for vmaster slave (%d!=%d)\n",
+-			  *step_to_check, step);
+		return -1;
+	}
+
+	*step_to_check = step;
+	return -tlv[2] / step;
 }
 
 /* call kctl->put with the given value(s) */
-- 
2.11.0

Re: 4.13 regression: get_kctl_0dB_offset doesn't handle all possible callbacks

[Takashi Iwai]

On Sat, 14 Oct 2017 07:31:20 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Oct 14 2017 07:46, PaX Team wrote:
> > what KERNEXEC on i386 does is that it executes kernel code in its own 0-based
> > code segment hence the 'low' code addresses. due to the current logic that
> > checks for SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK in get_kctl_0dB_offset, this
> > callback address is instead treated as a data pointer (as apparently
> > SNDRV_CTL_ELEM_ACCESS_TLV_READ is also set) and since it's not a valid kernel
> > address, it causes a GPF under the UDEREF PaX feature (without UDEREF it'd
> > have been an oops since such low addresses are not mapped for kernel threads).
> 
> There're two ways to use 'struct snd_kcontrol.tlv'; constant array (=.p) an
> a handler (= .c). An 'access' flag in each member of
> 'struct snd_kcontrol.vd' represent which way is used for the member.
> Therefore, as long as checking the flag, the 'get_kctl_0dB_offset()' doesn't
> handle a pointer to the handler as a pointer to array of TLV container.
> 
> > on vanilla kernels all this is a silent read of kernel code bytes that are
> > then interpreted as the tlv[] array content, which is probably not what you
> > want either.
> 
> Yes. Current code include a bug of inappropriate condition statement for the
> above. As a result, it allows to handle a pointer to the handler as a
> pointer to TLV data. 
> 
> > as for fixing this, first the above mentioned assumption should be re-evaluated.
> > if it's considered correct then there is some logic bug in my case (i can help
> > debug it if you tell me what to do) otherwise the current pattern of
> > 
> >     if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK && callback == snd_hda_mixer_amp_tlv) {
> >       call wrapped function underneath snd_hda_mixer_amp_tlv
> >     } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
> >       treat unrecognized callback address as data ptr
> >     }
> > 
> > should be changed to
> > 
> >     if (SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK( {
> >       if (callback == snd_hda_mixer_amp_tlv) {
> >         call wrapped function underneath snd_hda_mixer_amp_tlv
> >       } else if (callback == others) {
> >         handle others, WARN/BUG/etc
> >       }
> >     } else if (SNDRV_CTL_ELEM_ACCESS_TLV_READ) {
> >       no longer treat unrecognized callback address as data ptr
> >     }
> 
> Good enough as a solution. Please test a patch in the end of this message.

Unfortunately it doesn't work.

This brown paper bug is triggered not because of the lack of the
check.  (Yes, it's safer to have a check, but this shouldn't have hit
at all.)  The problem is rather that the same map_slaves() is used
after assigning to vmaster for applying the function to mute or 0dB
initialization.  When a slave is linked to vmaster, the kctl is faked
and the ops are overloaded.  And yet the driver code wants to access
the raw kctl ops, and thus it always fails, and falls into the crack
of the overlooked condition.

The fix patch is attached below.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda - Fix incorrect of TLV callback check by set_fs()
 removal

The commit 99b5c5bb9a54 ("ALSA: hda - Remove the use of set_fs()")
converted the get_kctl_0dB_offset() call for killing set_fs() usage in
HD-audio codec code.  The conversion assumed that the TLV callback
used in HD-audio code is only snd_hda_mixer_amp() and applies the TLV
calculation locally.

Although this assumption is correct, and all slave kctls are actually
with that callback, the current code is still utterly buggy, doesn't
hit this condition and falls back to the next check.  It's because the
function gets called after adding slave kctls to vmaster.  By
assigning a slave kctl, it's faked inside vmaster code, and the whole
kctl ops are overridden.  Thus the callback op points to a different
value as we've thought.

More badly, the next fallback check is SNDRV_CTL_ELEM_ACCESS_TLV_READ
access bit, and this always hits for each kctl with TLV.  Then it
evaluates the callback function pointer wrongly as if it were a TLV
array, which eventually triggers an Oops like:
 PAX: suspicious general protection fault: 0000 [#1] PREEMPT SMP
 EIP: 0060:[<009bbd2d>] init_slave_0dB+0x2d/0xa0
 Call Trace:
  [<009bb469>] map_slaves+0xb9/0xe0
  [<009bce0e>] __snd_hda_add_vmaster+0xde/0x110
  [<009c82f0>] snd_hda_gen_build_controls+0x1a0/0x1b0
  [<009d0a4d>] cx_auto_build_controls+0xd/0x70
  [<009bdf76>] snd_hda_codec_build_controls+0x186/0x1d0
  [<009b92ad>] hda_codec_driver_probe+0x6d/0xf0
  .....

For addressing the regression, this patch introduces a new helper to
vmaster code, snd_ctl_apply_vmaster_slaves().  This works similarly
like map_slaves() in hda_codec.c, it loops over the slave list of the
given master, and applies the given function to each slave.  Then the
initializer function receives the right kctl object and we can compare
the correct pointer instead of the faked one.

Also, for catching the similar breakage in future, give an error
message when the unexpected TLV callback is found and bail out
immediately.

Fixes: 99b5c5bb9a54 ("ALSA: hda - Remove the use of set_fs()")
Reported-by: PaX Team <pageexec@freemail.hu>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/control.h   |  3 ++
 sound/core/vmaster.c      | 31 +++++++++++++++
 sound/pci/hda/hda_codec.c | 97 +++++++++++++++++++++++++++--------------------
 3 files changed, 89 insertions(+), 42 deletions(-)

diff --git a/include/sound/control.h b/include/sound/control.h
index bd7246de58e7..a1f1152bc687 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -248,6 +248,9 @@ int snd_ctl_add_vmaster_hook(struct snd_kcontrol *kctl,
 			     void *private_data);
 void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only);
 #define snd_ctl_sync_vmaster_hook(kctl)	snd_ctl_sync_vmaster(kctl, true)
+int snd_ctl_apply_vmaster_slaves(struct snd_kcontrol *kctl,
+				 int (*func)(struct snd_kcontrol *, void *),
+				 void *arg);
 
 /*
  * Helper functions for jack-detection controls
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c
index 6c58e6f73a01..e43af18d4383 100644
--- a/sound/core/vmaster.c
+++ b/sound/core/vmaster.c
@@ -484,3 +484,34 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol *kcontrol, bool hook_only)
 		master->hook(master->hook_private_data, master->val);
 }
 EXPORT_SYMBOL_GPL(snd_ctl_sync_vmaster);
+
+/**
+ * snd_ctl_apply_vmaster_slaves - Apply function to each vmaster slave
+ * @kctl: vmaster kctl element
+ * @func: function to apply
+ * @arg: optional function argument
+ *
+ * Apply the function @func to each slave kctl of the given vmaster kctl.
+ * Returns 0 if successful, or a negative error code.
+ */
+int snd_ctl_apply_vmaster_slaves(struct snd_kcontrol *kctl,
+				 int (*func)(struct snd_kcontrol *, void *),
+				 void *arg)
+{
+	struct link_master *master;
+	struct link_slave *slave;
+	int err;
+
+	master = snd_kcontrol_chip(kctl);
+	err = master_init(master);
+	if (err < 0)
+		return err;
+	list_for_each_entry(slave, &master->slaves, list) {
+		err = func(&slave->slave, arg);
+		if (err < 0)
+			return err;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_ctl_apply_vmaster_slaves);
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 3db26c451837..a0989d231fd0 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -1803,36 +1803,6 @@ static int check_slave_present(struct hda_codec *codec,
 	return 1;
 }
 
-/* guess the value corresponding to 0dB */
-static int get_kctl_0dB_offset(struct hda_codec *codec,
-			       struct snd_kcontrol *kctl, int *step_to_check)
-{
-	int _tlv[4];
-	const int *tlv = NULL;
-	int val = -1;
-
-	if ((kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) &&
-	    kctl->tlv.c == snd_hda_mixer_amp_tlv) {
-		get_ctl_amp_tlv(kctl, _tlv);
-		tlv = _tlv;
-	} else if (kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_READ)
-		tlv = kctl->tlv.p;
-	if (tlv && tlv[0] == SNDRV_CTL_TLVT_DB_SCALE) {
-		int step = tlv[3];
-		step &= ~TLV_DB_SCALE_MUTE;
-		if (!step)
-			return -1;
-		if (*step_to_check && *step_to_check != step) {
-			codec_err(codec, "Mismatching dB step for vmaster slave (%d!=%d)\n",
--				   *step_to_check, step);
-			return -1;
-		}
-		*step_to_check = step;
-		val = -tlv[2] / step;
-	}
-	return val;
-}
-
 /* call kctl->put with the given value(s) */
 static int put_kctl_with_value(struct snd_kcontrol *kctl, int val)
 {
@@ -1847,19 +1817,58 @@ static int put_kctl_with_value(struct snd_kcontrol *kctl, int val)
 	return 0;
 }
 
-/* initialize the slave volume with 0dB */
-static int init_slave_0dB(struct hda_codec *codec,
-			  void *data, struct snd_kcontrol *slave)
+struct slave_init_arg {
+	struct hda_codec *codec;
+	int step;
+};
+
+/* initialize the slave volume with 0dB via snd_ctl_apply_vmaster_slaves() */
+static int init_slave_0dB(struct snd_kcontrol *kctl, void *_arg)
 {
-	int offset = get_kctl_0dB_offset(codec, slave, data);
-	if (offset > 0)
-		put_kctl_with_value(slave, offset);
+	struct slave_init_arg *arg = _arg;
+	int _tlv[4];
+	const int *tlv = NULL;
+	int step;
+	int val;
+
+	if (kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
+		if (kctl->tlv.c != snd_hda_mixer_amp_tlv) {
+			codec_err(arg->codec,
+				  "Unexpected TLV callback for slave %s:%d\n",
+				  kctl->id.name, kctl->id.index);
+			return 0; /* ignore */
+		}
+		get_ctl_amp_tlv(kctl, _tlv);
+		tlv = _tlv;
+	} else if (kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_READ)
+		tlv = kctl->tlv.p;
+
+	if (!tlv || tlv[0] != SNDRV_CTL_TLVT_DB_SCALE)
+		return 0;
+
+	step = tlv[3];
+	step &= ~TLV_DB_SCALE_MUTE;
+	if (!step)
+		return 0;
+	if (arg->step && arg->step != step) {
+		codec_err(arg->codec,
+			  "Mismatching dB step for vmaster slave (%d!=%d)\n",
+			  arg->step, step);
+		return 0;
+	}
+
+	arg->step = step;
+	val = -tlv[2] / step;
+	if (val > 0) {
+		put_kctl_with_value(kctl, val);
+		return val;
+	}
+
 	return 0;
 }
 
-/* unmute the slave */
-static int init_slave_unmute(struct hda_codec *codec,
-			     void *data, struct snd_kcontrol *slave)
+/* unmute the slave via snd_ctl_apply_vmaster_slaves() */
+static int init_slave_unmute(struct snd_kcontrol *slave, void *_arg)
 {
 	return put_kctl_with_value(slave, 1);
 }
@@ -1919,9 +1928,13 @@ int __snd_hda_add_vmaster(struct hda_codec *codec, char *name,
 	/* init with master mute & zero volume */
 	put_kctl_with_value(kctl, 0);
 	if (init_slave_vol) {
-		int step = 0;
-		map_slaves(codec, slaves, suffix,
-			   tlv ? init_slave_0dB : init_slave_unmute, &step);
+		struct slave_init_arg arg = {
+			.codec = codec,
+			.step = 0,
+		};
+		snd_ctl_apply_vmaster_slaves(kctl,
+					     tlv ? init_slave_0dB : init_slave_unmute,
+					     &arg);
 	}
 
 	if (ctl_ret)
-- 
2.14.2