[PATCH 2/2] powerpc/uprobes: Reject uprobe on a system call instruction
Naveen N. Rao
naveen.n.rao at linux.vnet.ibm.com
Fri Jan 28 22:30:25 AEDT 2022
On 2022-01-27 13:14, Nicholas Piggin wrote:
> Excerpts from Michael Ellerman's message of January 25, 2022 9:45 pm:
>> Nicholas Piggin <npiggin at gmail.com> writes:
>>> Per the ISA, a Trace interrupt is not generated for a system call
>>> [vectored] instruction. Reject uprobes on such instructions as we are
>>> not emulating a system call [vectored] instruction anymore.
>>
>> This should really be patch 1, otherwise there's a single commit
>> window
>> where we allow uprobes on sc but don't honour them.
>
> Yep true. I also messed up Naveen's attribution! Will re-send (or maybe
> Naveen would take over the series).
Yes, let me come up with a better, more complete patch for this.
>
>>
>>> Signed-off-by: Naveen N. Rao <naveen.n.rao at linux.vnet.ibm.com>
>>> [np: Switch to pr_info_ratelimited]
>>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>>> ---
>>> arch/powerpc/include/asm/ppc-opcode.h | 1 +
>>> arch/powerpc/kernel/uprobes.c | 6 ++++++
>>> 2 files changed, 7 insertions(+)
>>>
>>> diff --git a/arch/powerpc/include/asm/ppc-opcode.h
>>> b/arch/powerpc/include/asm/ppc-opcode.h
>>> index 9675303b724e..8bbe16ce5173 100644
>>> --- a/arch/powerpc/include/asm/ppc-opcode.h
>>> +++ b/arch/powerpc/include/asm/ppc-opcode.h
>>> @@ -411,6 +411,7 @@
>>> #define PPC_RAW_DCBFPS(a, b) (0x7c0000ac | ___PPC_RA(a) |
>>> ___PPC_RB(b) | (4 << 21))
>>> #define PPC_RAW_DCBSTPS(a, b) (0x7c0000ac | ___PPC_RA(a) |
>>> ___PPC_RB(b) | (6 << 21))
>>> #define PPC_RAW_SC() (0x44000002)
>>> +#define PPC_RAW_SCV() (0x44000001)
>>> #define PPC_RAW_SYNC() (0x7c0004ac)
>>> #define PPC_RAW_ISYNC() (0x4c00012c)
>>>
>>> diff --git a/arch/powerpc/kernel/uprobes.c
>>> b/arch/powerpc/kernel/uprobes.c
>>> index c6975467d9ff..3779fde804bd 100644
>>> --- a/arch/powerpc/kernel/uprobes.c
>>> +++ b/arch/powerpc/kernel/uprobes.c
>>> @@ -41,6 +41,12 @@ int arch_uprobe_analyze_insn(struct arch_uprobe
>>> *auprobe,
>>> if (addr & 0x03)
>>> return -EINVAL;
>>>
>>> + if (ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SC() ||
>>> + ppc_inst_val(ppc_inst_read(auprobe->insn)) == PPC_RAW_SCV()) {
>>
>> We should probably reject hypercall too?
>>
>> There's also a lot of reserved fields in `sc`, so doing an exact match
>> like this risks missing instructions that are badly formed but the CPU
>> will happily execute as `sc`.
>
> Yeah, scv as well has lev != 0 unsupported so should be excluded.
>>
>> We'd obviously never expect to see those in compiler generated code,
>> but
>> it'd still be safer to mask. We could probably just reject opcode 17
>> entirely.
Indeed, thanks.
>>
>> And I guess for a subsequent patch, but we should be rejecting some
>> others here as well shouldn't we? Like rfid etc.
>
> Traps under discussion I guess. For uprobe, rfid will be just another
> privilege fault. Is that dealt with somehow or do all privileged and
> illegal instructions also need to be excluded from stepping? (I assume
> we must handle that in a general way somehow)
Yes, this is all handled in our interrupt code if we emulate any of
those
privileged instructions. Otherwise, if a signal is generated, that would
be caught by uprobe_deny_signal().
Thanks,
Naveen
More information about the Linuxppc-dev
mailing list