linux-kernel - Re: Re: [PATCH v2 10/13] kprobes: Remove uneeded kernel dependency on struct arch_specific

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 14 Nov 2013 11:02:20 +0900
From:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To:	"Jon Medhurst (Tixy)" <tixy@...aro.org>
Cc:	David Long <dave.long@...aro.org>,
	linux-arm-kernel@...ts.infradead.org, Rabin Vincent <rabin@....in>,
	Oleg Nesterov <oleg@...hat.com>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: Re: [PATCH v2 10/13] kprobes: Remove uneeded kernel dependency
 on struct arch_specific_insn

(2013/11/14 2:13), Jon Medhurst (Tixy) wrote:
> On Tue, 2013-10-15 at 17:04 -0400, David Long wrote:
>> From: "David A. Long" <dave.long@...aro.org>
>>
>> Instead of depending on include/asm/kprobes.h to provide a dummy definition
>> for struct arch_specific_insn, do so in include/linux/kprobes.h.
> 
> That change description doesn't quite seem to quite make sense to me.
> 
> Anyway, what we're trying to do with this patch is to allow us to use
> arch_specific_insn for purposes additional to implementing kprobes. This
> patch enables that but I'm wary that the kprobes code assumes that ainsn
> is a struct arch_specific_insn, e.g. in linux/kernel/kprobes.c we have:
> 
> 	memcpy(&p->ainsn, &ap->ainsn, sizeof(struct arch_specific_insn));
> 
> Now, that code isn't compiled when kprobes isn't configured, but it
> seams to me to be safer if that was also changed to 
> 
> 	memcpy(&p->ainsn, &ap->ainsn, sizeof(p->ainsn));

This kind of cleanup looks good for me, but I don't agree to change
the type of the member (removing is OK) by Kconfig. If you want to
change the framework of kprobes and uprobes itself (unification),
I'm appreciate to discuss with you and uprobes people, because it
will involve all arch dependent code change, *NOT ONLY* the ARM issue.

> However, I also wonder if we should instead leave arch_specific_insn as
> a kprobes specific structure and on ARM define it in terms of a new more
> generic 'struct probe_insn'? The drawback with that is that we'd
> probably end up with a struct just containing a single member which
> seems a bit redundant:
>
> struct arch_specific_insn {
> 	struct probe_insn pinsn;
> };

I also disagree it. If you have a plan to integrate uprobes and kprobes
arch specific code, please share it with us. I'm happy to work with you.
There are already many maintainers on each feature who is responsible for
it (even it is a piece of code), and scripts/get_maintainers.pl gives you
who are.

Srikar, Oleg, I think it's a good time to merge such arch_specific mechanism
of uprobes and kprobes. Would you think we can do similar thing on x86 too?

Thank you,


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@...achi.com


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