Re: [RFC PATCH for 4.18 1/2] rseq: use __u64 for rseq_cs fields, validate abort_ip < TASK_SIZE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 2, 2018 at 2:03 PM Mathieu Desnoyers
<mathieu.desnoyers@xxxxxxxxxxxx> wrote:
>
>         /* Ensure that abort_ip is not in the critical section. */
>         if (rseq_cs->abort_ip - rseq_cs->start_ip < rseq_cs->post_commit_offset)
>                 return -EINVAL;
> ...
> What underflow issues are you concerned with ?

That.

Looking closer, it looks like what you want to do is

     if (rseq_cs->abort_ip >= rseq_cs->start_ip && rseq_cs->abort_ip <
rseq_cs->start_ip + rseq_cs->post_commit_offset)

but you're not actually verifying that the range you're testing is
even vlid, because "rseq_cs->start_ip + rseq_cs->post_commit_offset"
could be something invalid that overflowed (or, put another way, the
subtraction you did on both sides to get the simplified version
underflowed).

So to actually get the range check you want, you should check the
overflow/underflow condition. Maybe it ends up being

        if (rseq_cs->start_ip + rseq_cs->post_commit_offset < rseq_cs->start_ip)
                return -EINVAL;

after which your simplified conditional looks fine.

But I think you should also do

        if (rseq_cs->start_ip + rseq_cs->post_commit_offset > TASK_SIZE)
                return -EINVAL;

to make sure the range is valid in the first place.

             Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux