Re: [PATCH] tty: n_gsm: bound the EA scan in gsm_control_rls()

[Greg Kroah-Hartman]

On Sat, Jun 06, 2026 at 01:03:23PM -0400, Michael Bommarito wrote:
> gsm_control_rls() walks the control message looking for the end of the
> EA-encoded address, but it dereferences each byte before checking the
> remaining length:
> 
> 	int len = clen;
> 	const u8 *dp = data;
> 
> 	while (gsm_read_ea(&addr, *dp++) == 0) {
> 		len--;
> 		if (len == 0)
> 			return;
> 	}
> 
> When a malformed RLS control message decodes to a control length of
> zero, len starts at 0, the loop reads *dp before testing len, len
> underflows to -1, and the "if (len == 0)" test never fires. The scan
> then keeps reading forward through gsm->buf (kmalloc(MAX_MRU + 1)) until
> it happens upon a byte whose EA bit is set, reading past the end of the
> allocation.
> 
> A two byte DLCI 0 UIH control frame carrying CMD_RLS with an encoded
> length of zero reaches this from the receive path. KASAN reports a
> slab-out-of-bounds read in gsm_control_rls() (inlined into
> gsm_dlci_command()) when the bytes that follow the message in gsm->buf
> do not terminate the scan before the end of the buffer.
> 
> The sibling control handlers gsm_control_modem() and gsm_dlci_data()
> already reject a too short message before walking it; gsm_control_rls()
> was missed. Bound the EA scan by the remaining length so a zero length
> message returns without dereferencing past the data.
> 
> Fixes: e1eaea46bb40 ("tty: n_gsm line discipline")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> ---
>  drivers/tty/n_gsm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index c13e050de83b1..4f8f2ce038bc8 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -1806,12 +1806,12 @@ static void gsm_control_rls(struct gsm_mux *gsm, const u8 *data, int clen)
>  	int len = clen;
>  	const u8 *dp = data;
>  
> -	while (gsm_read_ea(&addr, *dp++) == 0) {
> +	while (len > 0) {
> +		if (gsm_read_ea(&addr, *dp++))
> +			break;
>  		len--;
> -		if (len == 0)
> -			return;
>  	}
> -	/* Must be at least one byte following ea */
> +	/* Must be at least one byte following the EA */
>  	len--;
>  	if (len <= 0)
>  		return;
> 
> base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8

While it's fun to throw LLMs at this codebase, how was this actually
tested?  there are loads of documented "problems" with this file, but in
the real world, with real devices, it works fine so without testing of
the code with those devices, we have to be careful with changing
anything here.

thanks,

greg k-h