Re: [PATCH] arch/tile: provide PT_FLAGS_COMPAT value in pt_regs

[Chris Metcalf]

On 12/13/2012 10:49 AM, Oleg Nesterov wrote:
> On 12/13, Chris Metcalf wrote:
>> On 12/12/2012 6:43 PM, Oleg Nesterov wrote:
>>> And. arch/tile/kernel/ptrace.c:arch_ptrace() does
>>>
>>> 	case PTRACE_SETOPTIONS:
>>> 		/* Support TILE-specific ptrace options. */
>>> 		child->ptrace &= ~PT_TRACE_MASK_TILE;
>>> 		tmp = data & PTRACE_O_MASK_TILE;
>>> 		data &= ~PTRACE_O_MASK_TILE;
>>>
>>> AFAICS we need something like BUILD_BUG_ON(PTRACE_O_MASK_TILE & PTRACE_O_MASK),
>> I don't think so.  These are disjoint namespaces anyway.
>> PTRACE_O_MASK_TILE is for the actual PTRACE_SETOPTIONS ABI values.
> Yes, and thus it should not intersect with the generic PTRACE_O_MASK, no?

Yes, I misunderstood your original suggestion (I read PTRACE_O_MASK as PT_TRACE_MASK_TILE - oops).  You're quite right that it's a good build bug; I'll add it to the cleanup patch that will also move the task->ptrace bit clear.

>>> 		ret = ptrace_request(child, request, addr, data);
>>> 		if (tmp & PTRACE_O_TRACEMIGRATE)
>>> 			child->ptrace |= PT_TRACE_MIGRATE;
>>>
>>> this also needs "ret == 0" check
>> The question is, what happens if we pass some illegal bit to the generic
>> ptrace_request(), and also pass a valid PTRACE_O_MASK_TILE bit?
>> Currently we set the tile-specific bit, then report the error.
>> This is consistent with how ptrace_setoptions() handles a mix of legal and
>> illegal bits.
> But ptrace_setoptions() returns EINVAL? it doesn't accept illegal bits.

It does return EINVAL - but if it gets both legal and illegal bits, it honors all the legal bits first.  So we honor the PT_TRACE_MIGRATE bit in this code, even if ptrace_request() returns EINVAL.

> So I'd say it looks fine to me.

Thanks!  Should I convert that to a Reviewed-by or Acked-by on the patch?

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/