Re: [PATCH] riscv: probes: reject kprobes inside LR/SC sequences

[Nam Cao <namcao@linutronix.de>]

Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> writes:
> +/*
> + * A trap taken in the middle of an LR/SC sequence clears the load
> + * reservation, so an SC following the probed instruction would always
> + * fail and the enclosing retry loop would re-enter the breakpoint.
> + * Reject probes inside such a sequence.
> + *
> + * A constrained LR/SC loop (Zalrsc) is limited to 16 instructions placed
> + * sequentially in memory (64 bytes in the base ISA), so scanning back
> + * that far covers every possible enclosing sequence.
> + */
> +#define MAX_ATOMIC_CONTEXT_SIZE	64

What about C extension? Does this 64 still work?

> +
> +static bool __kprobes riscv_probe_insn_in_atomic(unsigned long addr)
> +{
> +	unsigned long tmp, offset, scan_start;
> +	bool in_atomic = false;
> +
> +	if (!kallsyms_lookup_size_offset(addr, NULL, &offset))
> +		return false;
> +
> +	tmp = addr - offset;				/* function entry */
> +
> +	if (offset > MAX_ATOMIC_CONTEXT_SIZE)
> +		scan_start = addr - MAX_ATOMIC_CONTEXT_SIZE;

What guarantees that 'scan_start' points to the beginning of an instruction?

> +	else
> +		scan_start = tmp;
> +
> +	/* advance to the scan window, keeping instruction alignment */
> +	while (tmp < scan_start)
> +		tmp += GET_INSN_LENGTH(*(u16 *)tmp);
> +
> +	/* scan the window, tracking whether an LR is still outstanding */
> +	while (tmp < addr) {
> +		if (GET_INSN_LENGTH(*(u16 *)tmp) == 4) {

Why is tmp casted to (u16 *)? An instruction can be 32 bits.

It still works, I think, but looks confusing.

> +			u32 insn = *(u32 *)tmp;

Reuse this insn for the GET_INSN_LENGTH() above?

Nam

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv