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.
Dave
--
John David Anglin dave.anglin@xxxxxxxx
--
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
