* Re: [Bug 218708] New: Off-by-one vulnerability when reading data from the n_gsm module
[not found] <bug-218708-208349@https.bugzilla.kernel.org/>
@ 2024-04-12 5:17 ` Greg KH
0 siblings, 0 replies; only message in thread
From: Greg KH @ 2024-04-12 5:17 UTC (permalink / raw)
To: Daniel Starke; +Cc: linux-serial, jirislaby
Daniel, can you take a look at the bugzilla report below? There is lots
of "chatter" about the n_gsm code right now for obvious reasons, and I
have reports that there is at least one outstanding bug in the codebase
that can be triggered by userspace, perhaps this is that issue?
If not looking at it either way would be great if you could, thanks!
greg k-h
On Thu, Apr 11, 2024 at 01:56:38AM +0000, bugzilla-daemon@kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=218708
>
> Bug ID: 218708
> Summary: Off-by-one vulnerability when reading data from the
> n_gsm module
> Product: Linux
> Version: unspecified
> Hardware: All
> OS: Linux
> Status: NEW
> Severity: high
> Priority: P3
> Component: Kernel
> Assignee: linux-kernel@kernel-bugs.kernel.org
> Reporter: j51569436@gmail.com
> CC: gregkh@linuxfoundation.org
> Regression: No
>
> An off-by-one vulnerability occurs in gsm0_receive and gsm1_receive. I'll focus
> on gsm0_receive for our discussion.
>
>
> [1] : Write the value to gsm->buf, then increment gsm->count by 1.
> [2] : If gsm->count == gsm->len is reached, stop reading.
>
>
> Writing a value to a buffer and then checking its length is typical of
> off-by-one vulnerabilities.
>
> ```c
> static void gsm0_receive(struct gsm_mux *gsm, u8 c)
> {
> unsigned int len;
>
> switch (gsm->state) {
> ...
> case GSM_DATA: /* Data */
> gsm->buf[gsm->count++] = c;//[1]
> if (gsm->count == gsm->len) {//[2]
> /* Calculate final FCS for UI frames over all data */
> if ((gsm->control & ~PF) != UIH) {
> gsm->fcs = gsm_fcs_add_block(gsm->fcs,
> gsm->buf,
> gsm->count);
> }
> gsm->state = GSM_FCS;
> }
> break;
> case GSM_FCS: /* FCS follows the packet */
> gsm->fcs = gsm_fcs_add(gsm->fcs, c);
> gsm->state = GSM_SSOF;
> break;
> case GSM_SSOF:
> gsm->state = GSM_SEARCH;
> if (c == GSM0_SOF)
> gsm_queue(gsm);
> else
> gsm->bad_size++;
> break;
> default:
> pr_debug("%s: unhandled state: %d
> ", __func__, gsm->state);
> break;
> }
> }
> ```
>
> - `gsm->count == gsm->len` should be changed to `(gsm->count+1) == gsm->len`
>
> --
> You may reply to this email to add a comment.
>
> You are receiving this mail because:
> You are on the CC list for the bug.
^ permalink raw reply [flat|nested] only message in thread