Skip to contentSkip to conversation

New: Off-by-one vulnerability when reading data from the n_gsm module

Bug 218708 Greg KH 1 participant 1 message Apr 12, 2024 1 message not in the archive TTY layer and serial drivers

lore
Share & tools
lore

Greg KH

author maintainer
#1 lore raw
In reply to a message not in the archive

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

Quoted 74 lines · On Thu, Apr 11, 2024 at 01:56:38AM +0000, [email protected] 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: [email protected]
          Reporter: [email protected]
                CC: [email protected]
        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

Signature · 4 lines
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.