Messages in this thread | | | From | Linus Torvalds <> | Date | Mon, 2 Jul 2018 14:20:16 -0700 | Subject | Re: [RFC PATCH for 4.18 1/2] rseq: use __u64 for rseq_cs fields, validate abort_ip < TASK_SIZE |
| |
On Mon, Jul 2, 2018 at 2:03 PM Mathieu Desnoyers <mathieu.desnoyers@efficios.com> 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
| |