----- On Dec 20, 2015, at 1:31 PM, John David Anglin dave.anglin@xxxxxxxx wrote:
> On 2015-12-20, at 10:49 AM, Helge Deller wrote:
>
>>>>> /* Check if delay branch uses "copy %rX,%r20" */
>>>>> + if ((opcode & 0xff00ffff) == 0x08000254) {
>>>>> + source_reg = (opcode >> 16) & 31;
>>>>
>>>> Can you explain the reasoning behind the & 31 mask ?
>>>> I'm no parisc expert, but this seems rather odd.
>>>> Do you really mean "% 31" which translates to "& 5" ?
>>>> It would make more sense since there are 32 "gr"
>>>> registers. With & 31, a carefully crafted opcode
>>>> could overflow the gr[32] array, and cause a kernel
>>>> overflow allowing to overwrite kernel memory
>>>> (security issue).
>>>
>>> Hrm, I got my masks temporarily mixed up (early morning
>>> here). This is why I always use constructs such as:
>>>
>>> #define GR_REGS_BITS 5
>>> #define NR_GR_REGS (1U << GR_REGS_BITS)
>>> #define GR_REGS_MASK (NR_GR_REGS - 1)
>>>
>>> and then
>>>
>>> v & GR_REGS_MASK;
>>>
>>> Which makes everything super-obvious. The & 31 mask
>>> seems therefore technically correct.
>>
>> Good. I was struggling with your comments as well :-)
>>
>>
>>> The paragraph below still holds though:
>>>
>>>>
>>>> If it's the case, then it would also be good to
>>>> check that the register selector within the opcode
>>>> is not larger than 31 (e.g. applying to fr or sr
>>>> register), rather than applying the modulo and
>>>> assuming it's a gr and corrupt userspace state.
>>
>> I'll check that.
>
> The register field cannot be larger than 31. A fr or sr
> regster can't be used in a LDO "copy" instruction.
Independently of the instruction set, if an application
pulls the carpet from under the kernel from a concurrent
thread (or possibly ptrace) by forging an invalid
instruction after the system call has started executing,
it would be good to catch it and report it with a printk,
rather than blindly expecting that some bits should always
be 0.
Thanks,
Mathieu
>
> Dave
> --
> John David Anglin dave.anglin@xxxxxxxx
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
