ALSA: control: Add input validation

This patch adds a new feature to enable the validation of input data
to control elements in the ALSA core side.  When
CONFIG_SND_CTL_INPUT_VALIDATION is set, ALSA core verifies whether the
each input value via control API is in the defined ranges, also checks
whether it's aligned to the defined steps.  If an invalid value is
detected, ALSA core returns -EINVAL error immediately without passing
further to the driver's callback.  So this is a kind of hardening for
(badly written) drivers that have no proper error checks, at the cost
of a slight performance overhead.

Technically seen, this reuses a part of the existing validation code
for CONFIG_SND_CTL_DEBUG case with a slight modification to suppress
error prints for the input validation.

Link: https://lore.kernel.org/r/20220609120219.3937-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index 5289bb29131badc4d84db8168af9593a90d666b3..12990d9a4dffe76d7e321f780e181dee71fb8aef 100644 (file)
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -188,6 +188,16 @@ config SND_PCM_XRUN_DEBUG
          sound clicking when system is loaded, it may help to determine
          the process or driver which causes the scheduling gaps.
 
+config SND_CTL_INPUT_VALIDATION
+       bool "Validate input data to control API"
+       help
+         Say Y to enable the additional validation for the input data to
+         each control element, including the value range checks.
+         An error is returned from ALSA core for invalid inputs without
+         passing to the driver.  This is a kind of hardening for drivers
+         that have no proper error checks, at the cost of a slight
+         performance overhead.
+
 config SND_CTL_DEBUG
        bool "Enable debugging feature for control API"
        depends on SND_DEBUG

diff --git a/sound/core/control.c b/sound/core/control.c
index 559398891eb9757035e320bfede1e556b77b124d..fa04a92331554e487ad351b9b87208e035e952d9 100644 (file)
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -982,7 +982,7 @@ static void fill_remaining_elem_value(struct snd_ctl_elem_value *control,
 static int sanity_check_int_value(struct snd_card *card,
                                  const struct snd_ctl_elem_value *control,
                                  const struct snd_ctl_elem_info *info,
-                                 int i)
+                                 int i, bool print_error)
 {
        long long lval, lmin, lmax, lstep;
        u64 rem;
@@ -1016,21 +1016,23 @@ static int sanity_check_int_value(struct snd_card *card,
        }
 
        if (lval < lmin || lval > lmax) {
-               dev_err(card->dev,
-                       "control %i:%i:%i:%s:%i: value out of range %lld (%lld/%lld) at count %i\n",
-                       control->id.iface, control->id.device,
-                       control->id.subdevice, control->id.name,
-                       control->id.index, lval, lmin, lmax, i);
+               if (print_error)
+                       dev_err(card->dev,
+                               "control %i:%i:%i:%s:%i: value out of range %lld (%lld/%lld) at count %i\n",
+                               control->id.iface, control->id.device,
+                               control->id.subdevice, control->id.name,
+                               control->id.index, lval, lmin, lmax, i);
                return -EINVAL;
        }
        if (lstep) {
                div64_u64_rem(lval, lstep, &rem);
                if (rem) {
-                       dev_err(card->dev,
-                               "control %i:%i:%i:%s:%i: unaligned value %lld (step %lld) at count %i\n",
-                               control->id.iface, control->id.device,
-                               control->id.subdevice, control->id.name,
-                               control->id.index, lval, lstep, i);
+                       if (print_error)
+                               dev_err(card->dev,
+                                       "control %i:%i:%i:%s:%i: unaligned value %lld (step %lld) at count %i\n",
+                                       control->id.iface, control->id.device,
+                                       control->id.subdevice, control->id.name,
+                                       control->id.index, lval, lstep, i);
                        return -EINVAL;
                }
        }
@@ -1038,15 +1040,13 @@ static int sanity_check_int_value(struct snd_card *card,
        return 0;
 }
 
-/* perform sanity checks to the given snd_ctl_elem_value object */
-static int sanity_check_elem_value(struct snd_card *card,
-                                  const struct snd_ctl_elem_value *control,
-                                  const struct snd_ctl_elem_info *info,
-                                  u32 pattern)
+/* check whether the all input values are valid for the given elem value */
+static int sanity_check_input_values(struct snd_card *card,
+                                    const struct snd_ctl_elem_value *control,
+                                    const struct snd_ctl_elem_info *info,
+                                    bool print_error)
 {
-       size_t offset;
-       int i, ret = 0;
-       u32 *p;
+       int i, ret;
 
        switch (info->type) {
        case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
@@ -1054,7 +1054,8 @@ static int sanity_check_elem_value(struct snd_card *card,
        case SNDRV_CTL_ELEM_TYPE_INTEGER64:
        case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
                for (i = 0; i < info->count; i++) {
-                       ret = sanity_check_int_value(card, control, info, i);
+                       ret = sanity_check_int_value(card, control, info, i,
+                                                    print_error);
                        if (ret < 0)
                                return ret;
                }
@@ -1063,6 +1064,23 @@ static int sanity_check_elem_value(struct snd_card *card,
                break;
        }
 
+       return 0;
+}
+
+/* perform sanity checks to the given snd_ctl_elem_value object */
+static int sanity_check_elem_value(struct snd_card *card,
+                                  const struct snd_ctl_elem_value *control,
+                                  const struct snd_ctl_elem_info *info,
+                                  u32 pattern)
+{
+       size_t offset;
+       int ret;
+       u32 *p;
+
+       ret = sanity_check_input_values(card, control, info, true);
+       if (ret < 0)
+               return ret;
+
        /* check whether the remaining area kept untouched */
        offset = value_sizes[info->type] * info->count;
        offset = DIV_ROUND_UP(offset, sizeof(u32));
@@ -1249,6 +1267,17 @@ static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file,
 
        snd_ctl_build_ioff(&control->id, kctl, index_offset);
        result = snd_power_ref_and_wait(card);
+       /* validate input values */
+       if (IS_ENABLED(CONFIG_SND_CTL_INPUT_VALIDATION) && !result) {
+               struct snd_ctl_elem_info info;
+
+               memset(&info, 0, sizeof(info));
+               info.id = control->id;
+               result = __snd_ctl_elem_info(card, kctl, &info, NULL);
+               if (!result)
+                       result = sanity_check_input_values(card, control, &info,
+                                                          false);
+       }
        if (!result)
                result = kctl->put(kctl, control);
        snd_power_unref(card);