On 2015-12-21 9:42 AM, Mathieu Desnoyers wrote:
----- 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.
The mask should be 0xffe0ffff.
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
