lkml.org 
[lkml]   [2019]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] cpumask: Introduce possible_cpu_safe()
    On Thu, Apr 04, 2019 at 01:02:19PM +0300, Dan Carpenter wrote:
    > There have been two cases recently where we pass user a controlled "cpu"
    > to possible_cpus(). That's not allowed. If it's invalid, it will
    > trigger a WARN_ONCE() and an out of bounds read which could result in an
    > Oops.

    > +/**
    > + * cpumask_test_cpu_safe - test for a cpu in a cpumask
    > + * @cpu: cpu number
    > + * @cpumask: the cpumask pointer
    > + *
    > + * Returns 1 if @cpu is valid and set in @cpumask, else returns 0
    > + */
    > +static inline int cpumask_test_cpu_safe(int cpu, const struct cpumask *cpumask)
    > +{
    > + if ((unsigned int)cpu >= nr_cpu_ids)
    > + return 0;
    > + cpu = array_index_nospec(cpu, NR_CPUS);

    That should be:

    cpu = array_index_nospec(cpu, nr_cpu_ids);

    NR_CPUS might still be out-of-bounds for dynamically allocated cpumasks.

    > + return test_bit(cpu, cpumask_bits(cpumask));
    > +}

    That said; I don't particularly like this interface not its naming, how
    about something like:

    static inline unsigned int cpumask_validate_cpu(unsigned int cpu)
    {
    if (cpu >= nr_cpumask_bits)
    return nr_cpumask_bits;
    return array_index_nospec(cpu, nr_cpumask_bits);
    }

    Which you can then use like:

    cpu = cpumask_validate_cpu(user_cpu);
    if (cpu >= nr_cpu_ids)
    return -ENORANGE;

    /* @cpu is valid, do what needs doing */

    \
     
     \ /
      Last update: 2019-04-04 12:45    [W:2.511 / U:0.216 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site