On Tue, 24 Oct 2017 09:42:54 +0200,
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
>
> The patch below does not apply to the 4.13-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@xxxxxxxxxxxxxxx>.
I tried cherry-pick and it worked. The catch is that you need to
apply the commit 6bf88a343db2b3c160edf9b82a74966b31cc80bd before this
one.
Could you verify that?
thanks,
Takashi
>
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> >From a91d66129fb9bcead12af3ed2008d6ddbf179509 Mon Sep 17 00:00:00 2001
> From: Takashi Iwai <tiwai@xxxxxxx>
> Date: Mon, 16 Oct 2017 11:39:28 +0200
> Subject: [PATCH] ALSA: hda - Fix incorrect TLV callback check introduced
> during 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; it
> 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, the slave kctl object is faked inside
> vmaster code, and the whole kctl ops are overridden. Thus the
> callback op points to a different value from what we've assumed.
>
> More badly, as reported by the KERNEXEC and UDEREF features of PaX,
> the code flow turns into the unexpected pitfall. 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. Although currently its
> side-effect is fairly limited, this incorrect reference may lead to an
> unpleasant result.
>
> For addressing the regression, this patch introduces a new helper to
> vmaster code, snd_ctl_apply_vmaster_slaves(). This works similarly
> like the existing 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@xxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.13
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
>
> 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 b6cf9684c2ec..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)
>