RE: [PATCH 1/1] n_gsm: Use array_index_nospec() with index that comes from userspace

From: Starke, Daniel
Date: Wed Apr 12 2023 - 02:03:58 EST


> dc.channel used for indexing comes directly from copy_from_user(). Use
> array_index_nospec() to mitigate speculative side-channel.
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Link: https://lore.kernel.org/linux-serial/64306d13.ONswMlyWlVKLGkKR%25lkp@xxxxxxxxx/
> Fixes: afe3154ba87e ("tty: n_gsm: add ioctl for DLC config via ldisc handle")
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
> drivers/tty/n_gsm.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index b7e1369a035c..21985432794f 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -42,6 +42,7 @@
> #include <linux/ctype.h>
> #include <linux/mm.h>
> #include <linux/math.h>
> +#include <linux/nospec.h>
> #include <linux/string.h>
> #include <linux/slab.h>
> #include <linux/poll.h>
> @@ -3718,8 +3719,8 @@ static int gsmld_ioctl(struct tty_struct *tty, unsigned int cmd,
> struct gsm_config_ext ce;
> struct gsm_dlci_config dc;
> struct gsm_mux *gsm = tty->disc_data;
> + unsigned int base, addr;
> struct gsm_dlci *dlci;
> - unsigned int base;
>
> switch (cmd) {
> case GSMIOC_GETCONF:
> @@ -3748,9 +3749,10 @@ static int gsmld_ioctl(struct tty_struct *tty, unsigned int cmd,
> return -EFAULT;
> if (dc.channel == 0 || dc.channel >= NUM_DLCI)
> return -EINVAL;
> - dlci = gsm->dlci[dc.channel];
> + addr = array_index_nospec(dc.channel, NUM_DLCI);
> + dlci = gsm->dlci[addr];
> if (!dlci) {
> - dlci = gsm_dlci_alloc(gsm, dc.channel);
> + dlci = gsm_dlci_alloc(gsm, addr);
> if (!dlci)
> return -ENOMEM;
> }
> @@ -3763,9 +3765,10 @@ static int gsmld_ioctl(struct tty_struct *tty, unsigned int cmd,
> return -EFAULT;
> if (dc.channel == 0 || dc.channel >= NUM_DLCI)
> return -EINVAL;
> - dlci = gsm->dlci[dc.channel];
> + addr = array_index_nospec(dc.channel, NUM_DLCI);
> + dlci = gsm->dlci[addr];
> if (!dlci) {
> - dlci = gsm_dlci_alloc(gsm, dc.channel);
> + dlci = gsm_dlci_alloc(gsm, addr);
> if (!dlci)
> return -ENOMEM;
> }

Reviewed-by: Daniel Starke <daniel.starke@xxxxxxxxxxx>