Skip to content

Potential out-of-bounds access in myk(): thread_id may exceed NCPU while ks[] is sized by NCPU #38

@shaoyihao0517

Description

@shaoyihao0517

Hi, while reading the thread/runtime initialization code, I noticed a potential indexing issue around myk() and thread_id, and I wanted to check whether this is an intended invariant or a possible bug.

Relevant code

#define NCPU        256     /* max number of cpus */
#define NTHREAD     512     /* max number of threads */

/* an array of kthreads (thread_count/maxks in total) */
struct kthread ks[NCPU];

static inline struct kthread *myk(void)
{
    return &ks[this_thread_id()];
}

static inline unsigned int this_thread_id(void)
{
    return perthread_read(thread_id);
}

int thread_init_perthread(void)
{
    int ret;
    unsigned int thread_id;

    static __thread bool internal_thread_init_done;
    if (internal_thread_init_done)
        return 0;
    internal_thread_init_done = true;

    spin_lock(&thread_lock);
    if (thread_count >= NTHREAD) {
        spin_unlock(&thread_lock);
        log_err("thread: hit thread limit of %d\n", NTHREAD);
        return -ENOSPC;
    }
    thread_id = thread_count++;
    spin_unlock(&thread_lock);

    ret = thread_alloc_perthread(thread_id, 0);
    if (ret)
        return ret;

    perthread_store(thread_numa_node, 0);
    perthread_store(thread_id, thread_id);

    log_info("thread: created thread %d [tid: %d]", thread_id, thread_gettid());
    return 0;
}

Concern

thread_init_perthread() appears to assign thread_id from the range [0, NTHREAD), while ks[] is only sized as NCPU.

That means if more than NCPU threads are initialized, this_thread_id() may return a value >= NCPU, and then: return &ks[this_thread_id()]; would index past the end of ks[].

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions