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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
