[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200903270412.n2R4CG7o054009@www262.sakura.ne.jp>
Date: Fri, 27 Mar 2009 13:12:16 +0900
From: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To: pavel@....cz, jmorris@...ei.org
Cc: linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: TOMOYO in linux-next
Hello.
James Morris wrote:
> As for the rest of the feedback, please work with the developers to fix
> any bugs or lack of documentation.
Thanks.
Pavel Machek wrote:
> Are those interfaces documented somewhere?
They are documented at http://tomoyo.sourceforge.jp/en/2.2.x/policy-reference.html .
> This is quite nasty. I don't think turning off enforcement in
> interrupt is good idea. ("fails open").
This is not "fails open". TOMOYO deals only operations which are allowed to
sleep (e.g. opening files, making directories). This in_interrupt() check is
for safety in case somebody who are not allowed to sleep called TOMOYO's
function by error.
> I'm not sure basing security on pids is good idea...
PID is used for reaching a domain which that PID is in, not for access control
decisions.
> Hmm, barrier is spelled otherwise, and I'm not sure I'd trust this:
>
> +struct tomoyo_path_info_with_data {
> + /* Keep "head" first, for this pointer is passed to tomoyo_free(). */
> + struct tomoyo_path_info head;
> + char bariier1[16]; /* Safeguard for overrun. */
>
> I guess constants should be used here:
Oh, typo, thanks.
I think there is no need to use #define here, for nobody accesses
barrier1/barrier2.
> +#ifdef TOMOYO_DEBUG_DOMAIN_UNDELETE
> + if (domain2->is_deleted != 255)
> + printk(KERN_DEBUG
> + "Marked %p as non undeletable\n",
> + domain2);
> +#endif
> + domain2->is_deleted = 255;
>
> (I don't know why we want undelete in tomoyo.)
This "undelete domain" feature was introduced to allow administrators switch
domain policy periodically.
> If it contains copyright, it should contain copyright. It probably
> should not contain version numbers.
TOMOYO's management tools want /sys/kernel/security/tomoyo/version .
> Can we get an interface that does not need as many strings/ as much
> string parsing?
A plain text interface splitted by ' ' and '\n' is cleaner than introducing
binary interface. (TOMOYO uses \040 for ' ' and \012 for '\n'. No worry for
' ' and '\n' in pathnames.)
> That's my main complaint: Documentation.*tomoyo nor
> Documentation.*TOMOYO does not exist, still this adds a *lot* of new
> user<->kernel interfaces.
>
> New user<->kernel interaces should be documented and very carefuly
> reviewed; I don't think that happened here.
These user<->kernel interface are for TOMOYO's policy management tools,
not for general userland applications like /bin/bash /usr/sbin/sshd etc.
Regards.
----------------------------------------
TOMOYO: Fix a typo.
Signed-off-by: Kentaro Takeda <takedakn@...data.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@...data.co.jp>
---
security/tomoyo/common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.29-git1.orig/security/tomoyo/common.h
+++ linux-2.6.29-git1/security/tomoyo/common.h
@@ -55,7 +55,7 @@ struct tomoyo_path_info {
struct tomoyo_path_info_with_data {
/* Keep "head" first, for this pointer is passed to tomoyo_free(). */
struct tomoyo_path_info head;
- char bariier1[16]; /* Safeguard for overrun. */
+ char barrier1[16]; /* Safeguard for overrun. */
char body[TOMOYO_MAX_PATHNAME_LEN];
char barrier2[16]; /* Safeguard for overrun. */
};
--
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/
Powered by blists - more mailing lists