* [PATCH] LSM: add static to security_ops variable
@ 2010-02-07 11:24 wzt wzt
2010-02-07 14:14 ` Greg KH
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: wzt wzt @ 2010-02-07 11:24 UTC (permalink / raw)
To: linux-kernel; +Cc: sds, jmorris, eparis
security_ops was declared as a global variable, so other drivers or
kernel code can easily change its value, like:
extern struct security_operations *security_ops;
security_ops = NULL;
then insmod this driver immediately, it will get an oops. Other evil
drivers can aslo fake this variable as extern.
Signed-off-by: wzt <zhitong.wangzt@alibaba-inc.com>
---
security/security.c | 25 ++++++++++++++++++++++++-
security/selinux/hooks.c | 18 ++++++------------
2 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/security/security.c b/security/security.c
index 24e060b..781117d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -26,7 +26,12 @@ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
extern struct security_operations default_security_ops;
extern void security_fixup_ops(struct security_operations *ops);
-struct security_operations *security_ops; /* Initialized to NULL */
+static struct security_operations *security_ops; /* Initialized
to NULL */
+/*
+ * Minimal support for a secondary security module,
+ * just to allow the use of the capability module.
+ */
+static struct security_operations *secondary_ops;
static inline int verify(struct security_operations *ops)
{
@@ -63,6 +68,24 @@ int __init security_init(void)
return 0;
}
+void reset_secondary_ops(void)
+{
+ secondary_ops = security_ops;
+ if (!secondary_ops)
+ panic("SELinux: No initial security operations\n");
+}
+
+void reset_security_ops(void)
+{
+ /* Reset security_ops to the secondary module, dummy or capability. */
+ security_ops = secondary_ops;
+}
/* Save user chosen LSM */
static int __init choose_lsm(char *str)
{
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9a2ee84..9e8607e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -92,7 +92,9 @@
#define NUM_SEL_MNT_OPTS 5
extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
-extern struct security_operations *security_ops;
+extern void reset_secondary_ops(void);
+extern void reset_security_ops(void);
/* SECMARK reference count */
atomic_t selinux_secmark_refcount = ATOMIC_INIT(0);
@@ -126,12 +128,6 @@ int selinux_enabled = 1;
#endif
-/*
- * Minimal support for a secondary security module,
- * just to allow the use of the capability module.
- */
-static struct security_operations *secondary_ops;
-
/* Lists of inode and superblock security structures initialized
before the policy was loaded. */
static LIST_HEAD(superblock_security_head);
@@ -5672,9 +5668,8 @@ static __init int selinux_init(void)
0, SLAB_PANIC, NULL);
avc_init();
- secondary_ops = security_ops;
- if (!secondary_ops)
- panic("SELinux: No initial security operations\n");
+ reset_secondary_ops();
+
if (register_security(&selinux_ops))
panic("SELinux: Unable to register with kernel.\n");
@@ -5835,8 +5830,7 @@ int selinux_disable(void)
selinux_disabled = 1;
selinux_enabled = 0;
- /* Reset security_ops to the secondary module, dummy or capability. */
- security_ops = secondary_ops;
+ reset_security_ops();
/* Try to destroy the avc node cache */
avc_disable();
--
1.6.6.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] LSM: add static to security_ops variable 2010-02-07 11:24 [PATCH] LSM: add static to security_ops variable wzt wzt @ 2010-02-07 14:14 ` Greg KH 2010-02-16 14:57 ` Stephen Smalley 2010-02-19 11:44 ` Alexey Dobriyan 2 siblings, 0 replies; 12+ messages in thread From: Greg KH @ 2010-02-07 14:14 UTC (permalink / raw) To: wzt wzt; +Cc: linux-kernel, sds, jmorris, eparis On Sun, Feb 07, 2010 at 07:24:44PM +0800, wzt wzt wrote: > security_ops was declared as a global variable, so other drivers or > kernel code can easily change its value, like: > > extern struct security_operations *security_ops; > security_ops = NULL; > > then insmod this driver immediately, it will get an oops. Other evil > drivers can aslo fake this variable as extern. Evil drivers can do lots of things, if you can load a kernel module on the system, all bets are off. Just making this a private variable does not mean much. What external module are you trying to keep from using this variable? thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-07 11:24 [PATCH] LSM: add static to security_ops variable wzt wzt 2010-02-07 14:14 ` Greg KH @ 2010-02-16 14:57 ` Stephen Smalley 2010-02-19 11:40 ` wzt wzt 2010-02-19 11:44 ` Alexey Dobriyan 2 siblings, 1 reply; 12+ messages in thread From: Stephen Smalley @ 2010-02-16 14:57 UTC (permalink / raw) To: wzt wzt; +Cc: linux-kernel, jmorris, eparis, lsm, Greg Kroah-Hartman On Sun, 2010-02-07 at 19:24 +0800, wzt wzt wrote: > security_ops was declared as a global variable, so other drivers or > kernel code can easily change its value, like: > > extern struct security_operations *security_ops; > security_ops = NULL; > > then insmod this driver immediately, it will get an oops. Other evil > drivers can aslo fake this variable as extern. I'd support a patch along these lines (but with the changes below) for a different reason: at present, SELinux directly manipulates security_ops for the purpose of runtime disable support, whereas that ought to be handled by the security framework. > > Signed-off-by: wzt <zhitong.wangzt@alibaba-inc.com> > --- > security/security.c | 25 ++++++++++++++++++++++++- > security/selinux/hooks.c | 18 ++++++------------ > 2 files changed, 30 insertions(+), 13 deletions(-) > > diff --git a/security/security.c b/security/security.c > index 24e060b..781117d 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -26,7 +26,12 @@ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = > extern struct security_operations default_security_ops; > extern void security_fixup_ops(struct security_operations *ops); > > -struct security_operations *security_ops; /* Initialized to NULL */ > +static struct security_operations *security_ops; /* Initialized > to NULL */ > +/* > + * Minimal support for a secondary security module, > + * just to allow the use of the capability module. > + */ The comment is no longer accurate - secondary_ops was originally used by SELinux to call the "secondary" security module (capability or dummy), but that was replaced by direct calls to capability and the only remaining use is to save and restore the original security ops pointer value if SELinux is disabled by early userspace based on /etc/selinux/config. Further, if we support this directly in the security framework, then we can just use &default_security_ops for this purpose since that is now available. So I don't believe we need secondary_ops at all. > +static struct security_operations *secondary_ops; We don't need the above variable at all. > static inline int verify(struct security_operations *ops) > { > @@ -63,6 +68,24 @@ int __init security_init(void) > return 0; > } > > +void reset_secondary_ops(void) > +{ > + secondary_ops = security_ops; > + if (!secondary_ops) > + panic("SELinux: No initial security operations\n"); > +} We don't need the above function at all. > + > +void reset_security_ops(void) > +{ > + /* Reset security_ops to the secondary module, dummy or capability. */ The dummy module was removed so this can only be capability. > + security_ops = secondary_ops; This can just be: security_ops = &default_security_ops; > +} > > /* Save user chosen LSM */ > static int __init choose_lsm(char *str) > { > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 9a2ee84..9e8607e 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -92,7 +92,9 @@ > #define NUM_SEL_MNT_OPTS 5 > > extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm); > -extern struct security_operations *security_ops; > +extern void reset_secondary_ops(void); > +extern void reset_security_ops(void); The extern declaration for reset_security_ops() would properly go in include/linux/security.h for general use by security modules. > /* SECMARK reference count */ > atomic_t selinux_secmark_refcount = ATOMIC_INIT(0); > @@ -126,12 +128,6 @@ int selinux_enabled = 1; > #endif > > > -/* > - * Minimal support for a secondary security module, > - * just to allow the use of the capability module. > - */ > -static struct security_operations *secondary_ops; > - > /* Lists of inode and superblock security structures initialized > before the policy was loaded. */ > static LIST_HEAD(superblock_security_head); > @@ -5672,9 +5668,8 @@ static __init int selinux_init(void) > 0, SLAB_PANIC, NULL); > avc_init(); > > - secondary_ops = security_ops; > - if (!secondary_ops) > - panic("SELinux: No initial security operations\n"); > + reset_secondary_ops(); > + We don't need to save this value as it is available via &default_security_ops and there is now only one possible value (dummy module killed). > if (register_security(&selinux_ops)) > panic("SELinux: Unable to register with kernel.\n"); > > @@ -5835,8 +5830,7 @@ int selinux_disable(void) > selinux_disabled = 1; > selinux_enabled = 0; > > - /* Reset security_ops to the secondary module, dummy or capability. */ > - security_ops = secondary_ops; > + reset_security_ops(); So this is the only change needed here. > > /* Try to destroy the avc node cache */ > avc_disable(); -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-16 14:57 ` Stephen Smalley @ 2010-02-19 11:40 ` wzt wzt 2010-02-19 13:47 ` Stephen Smalley 0 siblings, 1 reply; 12+ messages in thread From: wzt wzt @ 2010-02-19 11:40 UTC (permalink / raw) To: Stephen Smalley; +Cc: linux-kernel, jmorris, eparis, lsm, Greg Kroah-Hartman I rewrite the patch, thx. --- include/linux/security.h | 2 ++ security/security.c | 7 ++++++- security/selinux/hooks.c | 14 ++------------ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/include/linux/security.h b/include/linux/security.h index 2c627d3..3a15b57 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -95,6 +95,8 @@ struct seq_file; extern int cap_netlink_send(struct sock *sk, struct sk_buff *skb); extern int cap_netlink_recv(struct sk_buff *skb, int cap); +extern void reset_security_ops(void); + #ifdef CONFIG_MMU extern unsigned long mmap_min_addr; extern unsigned long dac_mmap_min_addr; diff --git a/security/security.c b/security/security.c index 122b748..3e4c4bc 100644 --- a/security/security.c +++ b/security/security.c @@ -26,7 +26,7 @@ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = extern struct security_operations default_security_ops; extern void security_fixup_ops(struct security_operations *ops); -struct security_operations *security_ops; /* Initialized to NULL */ +static struct security_operations *security_ops; /* Initialized to NULL */ static inline int verify(struct security_operations *ops) { @@ -63,6 +63,11 @@ int __init security_init(void) return 0; } +void reset_security_ops(void) +{ + security_ops = &default_security_ops; +} + /* Save user chosen LSM */ static int __init choose_lsm(char *str) { diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9a2ee84..e9599fd 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -93,6 +93,7 @@ extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm); extern struct security_operations *security_ops; +extern struct security_operations default_security_ops; /* SECMARK reference count */ atomic_t selinux_secmark_refcount = ATOMIC_INIT(0); @@ -125,13 +126,6 @@ __setup("selinux=", selinux_enabled_setup); int selinux_enabled = 1; #endif - -/* - * Minimal support for a secondary security module, - * just to allow the use of the capability module. - */ -static struct security_operations *secondary_ops; - /* Lists of inode and superblock security structures initialized before the policy was loaded. */ static LIST_HEAD(superblock_security_head); @@ -5672,9 +5666,6 @@ static __init int selinux_init(void) 0, SLAB_PANIC, NULL); avc_init(); - secondary_ops = security_ops; - if (!secondary_ops) - panic("SELinux: No initial security operations\n"); if (register_security(&selinux_ops)) panic("SELinux: Unable to register with kernel.\n"); @@ -5835,8 +5826,7 @@ int selinux_disable(void) selinux_disabled = 1; selinux_enabled = 0; - /* Reset security_ops to the secondary module, dummy or capability. */ - security_ops = secondary_ops; + reset_security_ops(); /* Try to destroy the avc node cache */ avc_disable(); -- 1.6.5.3 On Tue, Feb 16, 2010 at 10:57 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On Sun, 2010-02-07 at 19:24 +0800, wzt wzt wrote: >> security_ops was declared as a global variable, so other drivers or >> kernel code can easily change its value, like: >> >> extern struct security_operations *security_ops; >> security_ops = NULL; >> >> then insmod this driver immediately, it will get an oops. Other evil >> drivers can aslo fake this variable as extern. > > I'd support a patch along these lines (but with the changes below) for a > different reason: at present, SELinux directly manipulates security_ops > for the purpose of runtime disable support, whereas that ought to be > handled by the security framework. > >> >> Signed-off-by: wzt <zhitong.wangzt@alibaba-inc.com> >> --- >> security/security.c | 25 ++++++++++++++++++++++++- >> security/selinux/hooks.c | 18 ++++++------------ >> 2 files changed, 30 insertions(+), 13 deletions(-) >> >> diff --git a/security/security.c b/security/security.c >> index 24e060b..781117d 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -26,7 +26,12 @@ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = >> extern struct security_operations default_security_ops; >> extern void security_fixup_ops(struct security_operations *ops); >> >> -struct security_operations *security_ops; /* Initialized to NULL */ >> +static struct security_operations *security_ops; /* Initialized >> to NULL */ >> +/* >> + * Minimal support for a secondary security module, >> + * just to allow the use of the capability module. >> + */ > > The comment is no longer accurate - secondary_ops was originally used by > SELinux to call the "secondary" security module (capability or dummy), > but that was replaced by direct calls to capability and the only > remaining use is to save and restore the original security ops pointer > value if SELinux is disabled by early userspace based > on /etc/selinux/config. Further, if we support this directly in the > security framework, then we can just use &default_security_ops for this > purpose since that is now available. So I don't believe we need > secondary_ops at all. > >> +static struct security_operations *secondary_ops; > > We don't need the above variable at all. > >> static inline int verify(struct security_operations *ops) >> { >> @@ -63,6 +68,24 @@ int __init security_init(void) >> return 0; >> } >> >> +void reset_secondary_ops(void) >> +{ >> + secondary_ops = security_ops; >> + if (!secondary_ops) >> + panic("SELinux: No initial security operations\n"); >> +} > > We don't need the above function at all. > >> + >> +void reset_security_ops(void) >> +{ >> + /* Reset security_ops to the secondary module, dummy or capability. */ > > The dummy module was removed so this can only be capability. > >> + security_ops = secondary_ops; > > This can just be: > security_ops = &default_security_ops; > >> +} >> >> /* Save user chosen LSM */ >> static int __init choose_lsm(char *str) >> { >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 9a2ee84..9e8607e 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -92,7 +92,9 @@ >> #define NUM_SEL_MNT_OPTS 5 >> >> extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm); >> -extern struct security_operations *security_ops; >> +extern void reset_secondary_ops(void); >> +extern void reset_security_ops(void); > > The extern declaration for reset_security_ops() would properly go in > include/linux/security.h for general use by security modules. > >> /* SECMARK reference count */ >> atomic_t selinux_secmark_refcount = ATOMIC_INIT(0); >> @@ -126,12 +128,6 @@ int selinux_enabled = 1; >> #endif >> >> >> -/* >> - * Minimal support for a secondary security module, >> - * just to allow the use of the capability module. >> - */ >> -static struct security_operations *secondary_ops; >> - >> /* Lists of inode and superblock security structures initialized >> before the policy was loaded. */ >> static LIST_HEAD(superblock_security_head); >> @@ -5672,9 +5668,8 @@ static __init int selinux_init(void) >> 0, SLAB_PANIC, NULL); >> avc_init(); >> >> - secondary_ops = security_ops; >> - if (!secondary_ops) >> - panic("SELinux: No initial security operations\n"); >> + reset_secondary_ops(); >> + > > We don't need to save this value as it is available via > &default_security_ops and there is now only one possible value (dummy > module killed). > >> if (register_security(&selinux_ops)) >> panic("SELinux: Unable to register with kernel.\n"); >> >> @@ -5835,8 +5830,7 @@ int selinux_disable(void) >> selinux_disabled = 1; >> selinux_enabled = 0; >> >> - /* Reset security_ops to the secondary module, dummy or capability. */ >> - security_ops = secondary_ops; >> + reset_security_ops(); > > So this is the only change needed here. > >> >> /* Try to destroy the avc node cache */ >> avc_disable(); > -- > Stephen Smalley > National Security Agency > > ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-19 11:40 ` wzt wzt @ 2010-02-19 13:47 ` Stephen Smalley 0 siblings, 0 replies; 12+ messages in thread From: Stephen Smalley @ 2010-02-19 13:47 UTC (permalink / raw) To: wzt wzt; +Cc: linux-kernel, jmorris, eparis, lsm, Greg Kroah-Hartman On Fri, 2010-02-19 at 19:40 +0800, wzt wzt wrote: > I rewrite the patch, thx. Patch description needs to be descriptive, e.g.: "Enhance the security framework to support resetting the active security module. This eliminates the need for direct use of the security_ops variable outside of security.c, so make security_ops static." Subject line could be more descriptive too, and likely just use security: as the prefix. You also need a Signed-off-by line, with a real name. Also, see the comments below. > --- > include/linux/security.h | 2 ++ > security/security.c | 7 ++++++- > security/selinux/hooks.c | 14 ++------------ > 3 files changed, 10 insertions(+), 13 deletions(-) > > diff --git a/security/security.c b/security/security.c > index 122b748..3e4c4bc 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -26,7 +26,7 @@ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = > extern struct security_operations default_security_ops; > extern void security_fixup_ops(struct security_operations *ops); > > -struct security_operations *security_ops; /* Initialized to NULL */ > +static struct security_operations *security_ops; /* Initialized > to NULL */ You can drop the /* Initialized to NULL */ comment since it is now static. > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 9a2ee84..e9599fd 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -93,6 +93,7 @@ > > extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm); > extern struct security_operations *security_ops; > +extern struct security_operations default_security_ops; We don't need this extern declaration. The next obvious cleanup would be to make default_security_ops static. That will require some code reorganization though - it is presently defined in capability.c and manipulated by security.c. -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-07 11:24 [PATCH] LSM: add static to security_ops variable wzt wzt 2010-02-07 14:14 ` Greg KH 2010-02-16 14:57 ` Stephen Smalley @ 2010-02-19 11:44 ` Alexey Dobriyan 2010-02-19 11:57 ` wzt wzt 2 siblings, 1 reply; 12+ messages in thread From: Alexey Dobriyan @ 2010-02-19 11:44 UTC (permalink / raw) To: wzt wzt; +Cc: linux-kernel, sds, jmorris, eparis On Sun, Feb 7, 2010 at 1:24 PM, wzt wzt <wzt.wzt@gmail.com> wrote: > security_ops was declared as a global variable, so other drivers or > kernel code can easily change its value, like: With your patch they still can. > extern struct security_operations *security_ops; > security_ops = NULL; > > then insmod this driver immediately, it will get an oops. Other evil > drivers can aslo fake this variable as extern. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-19 11:44 ` Alexey Dobriyan @ 2010-02-19 11:57 ` wzt wzt 2010-02-19 12:02 ` Alexey Dobriyan 0 siblings, 1 reply; 12+ messages in thread From: wzt wzt @ 2010-02-19 11:57 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: linux-kernel, sds, jmorris, eparis Maybe, but The attackers will use a complicated way to find the security_ops address, it's a barrier to attackers. LSM is security framework, we don't want the attackers can easily to break it. Just like the sys_call_table variable in kernel 2.4.x(global and writeable), evil drivers can extern the variable, then replace the Sys_X functions. On Fri, Feb 19, 2010 at 7:44 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: > On Sun, Feb 7, 2010 at 1:24 PM, wzt wzt <wzt.wzt@gmail.com> wrote: >> security_ops was declared as a global variable, so other drivers or >> kernel code can easily change its value, like: > > With your patch they still can. > >> extern struct security_operations *security_ops; >> security_ops = NULL; >> >> then insmod this driver immediately, it will get an oops. Other evil >> drivers can aslo fake this variable as extern. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-19 11:57 ` wzt wzt @ 2010-02-19 12:02 ` Alexey Dobriyan 2010-02-19 12:23 ` wzt wzt 0 siblings, 1 reply; 12+ messages in thread From: Alexey Dobriyan @ 2010-02-19 12:02 UTC (permalink / raw) To: wzt wzt; +Cc: linux-kernel, sds, jmorris, eparis On Fri, Feb 19, 2010 at 1:57 PM, wzt wzt <wzt.wzt@gmail.com> wrote: > Maybe, but The attackers will use a complicated way to find the > security_ops address, it's a barrier to attackers. It's not a barrier, it's garbage. Once you know the adress security_ops ended up at, you simply write to it. > LSM is security framework, we don't want the attackers can easily > to break it. LSM doesn't protect kernel from kernel. > Just like the sys_call_table variable in kernel 2.4.x(global and > writeable), evil drivers can extern the variable, then replace the > Sys_X functions. Not that easily, but they still can. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-19 12:02 ` Alexey Dobriyan @ 2010-02-19 12:23 ` wzt wzt 2010-02-19 12:27 ` Alexey Dobriyan 2010-02-19 13:12 ` Bernd Petrovitsch 0 siblings, 2 replies; 12+ messages in thread From: wzt wzt @ 2010-02-19 12:23 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: linux-kernel, sds, jmorris, eparis > It's not a barrier, it's garbage. Once you know the adress security_ops > ended up at, you simply write to it. How to find the security_ops address if the variable is static? Would you please make an example? > Not that easily, but they still can. That's why i suggest to make the variable to static, if you had wrote a rootkit, you will find that in kernel 2.4.x, there are many many rootkits, but in kernel 2.6.x, rootkit became fewer. Not all the kernel driver writers can master this method to find the variable's address. The patch also delete the secondary_ops variable. On Fri, Feb 19, 2010 at 8:02 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: > On Fri, Feb 19, 2010 at 1:57 PM, wzt wzt <wzt.wzt@gmail.com> wrote: >> Maybe, but The attackers will use a complicated way to find the >> security_ops address, it's a barrier to attackers. > > It's not a barrier, it's garbage. Once you know the adress security_ops > ended up at, you simply write to it. > >> LSM is security framework, we don't want the attackers can easily >> to break it. > > LSM doesn't protect kernel from kernel. > >> Just like the sys_call_table variable in kernel 2.4.x(global and >> writeable), evil drivers can extern the variable, then replace the >> Sys_X functions. > > Not that easily, but they still can. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-19 12:23 ` wzt wzt @ 2010-02-19 12:27 ` Alexey Dobriyan 2010-02-19 13:14 ` wzt wzt 2010-02-19 13:12 ` Bernd Petrovitsch 1 sibling, 1 reply; 12+ messages in thread From: Alexey Dobriyan @ 2010-02-19 12:27 UTC (permalink / raw) To: wzt wzt; +Cc: linux-kernel, sds, jmorris, eparis On Fri, Feb 19, 2010 at 2:23 PM, wzt wzt <wzt.wzt@gmail.com> wrote: >> It's not a barrier, it's garbage. Once you know the adress security_ops >> ended up at, you simply write to it. > > How to find the security_ops address if the variable is static? Would > you please make an example? See /proc/kallsyms . >> Not that easily, but they still can. > That's why i suggest to make the variable to static, if you had wrote > a rootkit, you will find that in kernel 2.4.x, there are many many > rootkits, but in kernel 2.6.x, rootkit became fewer. Not all the > kernel driver writers can master this method to find the variable's > address. Please. > The patch also delete the secondary_ops variable. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-19 12:27 ` Alexey Dobriyan @ 2010-02-19 13:14 ` wzt wzt 0 siblings, 0 replies; 12+ messages in thread From: wzt wzt @ 2010-02-19 13:14 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: linux-kernel, sds, jmorris, eparis security_ops is not static, so you can find the address with kallsysm, but you can try secondary_ops: static struct security_operations *secondary_ops = NULL; cat /proc/kallsysm|grep secondary_ops On Fri, Feb 19, 2010 at 8:27 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote: > On Fri, Feb 19, 2010 at 2:23 PM, wzt wzt <wzt.wzt@gmail.com> wrote: >>> It's not a barrier, it's garbage. Once you know the adress security_ops >>> ended up at, you simply write to it. >> >> How to find the security_ops address if the variable is static? Would >> you please make an example? > > See /proc/kallsyms . > >>> Not that easily, but they still can. >> That's why i suggest to make the variable to static, if you had wrote >> a rootkit, you will find that in kernel 2.4.x, there are many many >> rootkits, but in kernel 2.6.x, rootkit became fewer. Not all the >> kernel driver writers can master this method to find the variable's >> address. > > Please. > >> The patch also delete the secondary_ops variable. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] LSM: add static to security_ops variable 2010-02-19 12:23 ` wzt wzt 2010-02-19 12:27 ` Alexey Dobriyan @ 2010-02-19 13:12 ` Bernd Petrovitsch 1 sibling, 0 replies; 12+ messages in thread From: Bernd Petrovitsch @ 2010-02-19 13:12 UTC (permalink / raw) To: wzt wzt; +Cc: Alexey Dobriyan, linux-kernel, sds, jmorris, eparis On Fre, 2010-02-19 at 20:23 +0800, wzt wzt wrote: > > It's not a barrier, it's garbage. Once you know the adress security_ops > > ended up at, you simply write to it. > > How to find the security_ops address if the variable is static? Would > you please make an example? - Find the accessor function (which is surely non-static). - Find the address where writes the parameter. Other must decide, if it's more secure (as in "obscure") with the accessor funtion or not. Bernd -- Bernd Petrovitsch Email : bernd@petrovitsch.priv.at LUGA : http://www.luga.at ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-02-19 13:47 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-02-07 11:24 [PATCH] LSM: add static to security_ops variable wzt wzt 2010-02-07 14:14 ` Greg KH 2010-02-16 14:57 ` Stephen Smalley 2010-02-19 11:40 ` wzt wzt 2010-02-19 13:47 ` Stephen Smalley 2010-02-19 11:44 ` Alexey Dobriyan 2010-02-19 11:57 ` wzt wzt 2010-02-19 12:02 ` Alexey Dobriyan 2010-02-19 12:23 ` wzt wzt 2010-02-19 12:27 ` Alexey Dobriyan 2010-02-19 13:14 ` wzt wzt 2010-02-19 13:12 ` Bernd Petrovitsch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox