public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* systemtap broken by USER_NS for now
@ 2013-05-22 14:00 Alexander Y. Fomichev
  2013-05-22 21:40 ` David Smith
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander Y. Fomichev @ 2013-05-22 14:00 UTC (permalink / raw)
  To: systemtap

[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]

Hi

for now user_ns unconditionally requires
CONFIG_UIDGID_STRICT_TYPE_CHECKS and the latter in turn breaks things
this way:

typedef struct {
        uid_t val;
} kuid_t;

and so:

stap # ./probe.stp
In file included from /usr/share/systemtap/runtime/linux/task_finder.c:17:0,
                 from /usr/share/systemtap/runtime/linux/runtime.h:188,
                 from /usr/share/systemtap/runtime/runtime.h:24,
                 from
/tmp/.private/root/stap7M7WmA/stap_6ce999ddb8d32c70dcbe287153f6957f_16537_src.c:22:
/usr/share/systemtap/runtime/linux/task_finder2.c: In function
'__stp_utrace_attach_match_filename':
/usr/share/systemtap/runtime/linux/task_finder2.c:816:11: error:
incompatible types when assigning to type 'uid_t' from type 'kuid_t'
  tsk_euid = task_euid(tsk);
           ^
.....................................................................

It seems like fix is straightforward - just use from_kuid_munged to
get euid if CONFIG_UIDGID_STRICT_TYPE_CHECKS is defined. (really it
always works on latest kernels but breaks older ones). I've attached a
simple patch and it works for 3.9 (both with and without
STRICT_TYPE_CHECKS) but i'm not systemtap guy so i could be completely
wrong. Feel free to throw it out without reading :)

--
Best regards.
       Alexander Y. Fomichev <git.user@gmail.com>

[-- Attachment #2: uidgid_strict_type_check.patch --]
[-- Type: application/octet-stream, Size: 5734 bytes --]

diff -urNp a/runtime/linux/task_finder.c b/runtime/linux/task_finder.c
--- a/runtime/linux/task_finder.c	2013-05-21 15:51:55.000000000 +0400
+++ b/runtime/linux/task_finder.c	2013-05-21 16:45:47.634899524 +0400
@@ -23,6 +23,10 @@
 #ifndef STAPCONF_TASK_UID
 #include <linux/cred.h>
 #endif
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+#include <linux/cred.h>
+#include <linux/uidgid.h>
+#endif
 #include "syscall.h"
 #include "utrace_compatibility.h"
 #include "task_finder_map.c"
@@ -851,8 +855,12 @@ __stp_utrace_attach_match_filename(struc
 #ifdef STAPCONF_TASK_UID
 	tsk_euid = tsk->euid;
 #else
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+	tsk_euid = from_kuid_munged(current_user_ns(), task_euid(tsk));
+#else
 	tsk_euid = task_euid(tsk);
 #endif
+#endif
 	filelen = strlen(filename);
 	list_for_each(tgt_node, &__stp_task_finder_list) {
 		int rc;
@@ -1660,8 +1668,12 @@ stap_start_task_finder(void)
 #ifdef STAPCONF_TASK_UID
 		tsk_euid = tsk->euid;
 #else
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+		tsk_euid = from_kuid_munged(current_user_ns(), task_euid(tsk));
+#else
 		tsk_euid = task_euid(tsk);
 #endif
+#endif
 		mmpathlen = strlen(mmpath);
 		list_for_each(tgt_node, &__stp_task_finder_list) {
 			struct stap_task_finder_target *tgt;
diff -urNp a/runtime/linux/task_finder2.c b/runtime/linux/task_finder2.c
--- a/runtime/linux/task_finder2.c	2013-05-21 15:51:55.000000000 +0400
+++ b/runtime/linux/task_finder2.c	2013-05-21 16:45:47.638232907 +0400
@@ -9,6 +9,10 @@
 #ifndef STAPCONF_TASK_UID
 #include <linux/cred.h>
 #endif
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+#include <linux/cred.h>
+#include <linux/uidgid.h>
+#endif
 #include "syscall.h"
 #include "task_finder_map.c"
 #include "task_finder_vma.c"
@@ -813,8 +817,12 @@ __stp_utrace_attach_match_filename(struc
 #ifdef STAPCONF_TASK_UID
 	tsk_euid = tsk->euid;
 #else
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+	tsk_euid = from_kuid_munged(current_user_ns(), task_euid(tsk));
+#else
 	tsk_euid = task_euid(tsk);
 #endif
+#endif
 	filelen = strlen(filename);
 	list_for_each(tgt_node, &__stp_task_finder_list) {
 		int rc;
@@ -1699,8 +1707,12 @@ stap_start_task_finder(void)
 #ifdef STAPCONF_TASK_UID
 		tsk_euid = tsk->euid;
 #else
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+		tsk_euid = from_kuid_munged(current_user_ns(), task_euid(tsk));
+#else
 		tsk_euid = task_euid(tsk);
 #endif
+#endif
 		mmpathlen = strlen(mmpath);
 		list_for_each(tgt_node, &__stp_task_finder_list) {
 			struct stap_task_finder_target *tgt;
diff -urNp a/runtime/transport/control.c b/runtime/transport/control.c
--- a/runtime/transport/control.c	2013-05-21 15:51:54.000000000 +0400
+++ b/runtime/transport/control.c	2013-05-21 16:45:47.638232907 +0400
@@ -14,6 +14,10 @@
 #include "symbols.c"
 #include <linux/delay.h>
 #include <linux/poll.h>
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+#include <linux/cred.h>
+#include <linux/uidgid.h>
+#endif
 
 static _stp_mempool_t *_stp_pool_q;
 static struct list_head _stp_ctl_ready_q;
@@ -34,8 +38,12 @@ static ssize_t _stp_ctl_write_cmd(struct
 #ifdef STAPCONF_TASK_UID
 	uid_t euid = current->euid;
 #else
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+	uid_t euid = from_kuid_munged(current_user_ns(), current_euid());
+#else
 	uid_t euid = current_euid();
 #endif
+#endif
 
 	if (count < sizeof(u32))
 		return 0;
diff -urNp a/runtime/transport/debugfs.c b/runtime/transport/debugfs.c
--- a/runtime/transport/debugfs.c	2013-05-21 15:51:54.000000000 +0400
+++ b/runtime/transport/debugfs.c	2013-05-21 16:45:47.638232907 +0400
@@ -48,8 +48,8 @@ static int _stp_register_ctl_channel_fs(
 		return -1;
 	}
 
-	_stp_cmd_file->d_inode->i_uid = _stp_uid;
-	_stp_cmd_file->d_inode->i_gid = _stp_gid;
+	_stp_cmd_file->d_inode->i_uid = KUIDT_INIT(_stp_uid);
+	_stp_cmd_file->d_inode->i_gid = KGIDT_INIT(_stp_gid);
 
 	return 0;
 }
diff -urNp a/runtime/transport/relay_v2.c b/runtime/transport/relay_v2.c
--- a/runtime/transport/relay_v2.c	2013-05-21 15:51:54.000000000 +0400
+++ b/runtime/transport/relay_v2.c	2013-05-21 16:45:47.638232907 +0400
@@ -234,8 +234,8 @@ __stp_relay_create_buf_file_callback(con
 		file = NULL;
 	}
 	else if (file) {
-		file->d_inode->i_uid = _stp_uid;
-		file->d_inode->i_gid = _stp_gid;
+		file->d_inode->i_uid = KUIDT_INIT(_stp_uid);
+		file->d_inode->i_gid = KGIDT_INIT(_stp_gid);
 	}
 	return file;
 }
@@ -303,8 +303,8 @@ static int _stp_transport_data_fs_init(v
 		goto err;
 	}
 
-	_stp_relay_data.dropped_file->d_inode->i_uid = _stp_uid;
-	_stp_relay_data.dropped_file->d_inode->i_gid = _stp_gid;
+	_stp_relay_data.dropped_file->d_inode->i_uid = KUIDT_INIT(_stp_uid);
+	_stp_relay_data.dropped_file->d_inode->i_gid = KGIDT_INIT(_stp_gid);
 
 	/* Create "trace" file. */
 	npages = _stp_subbuf_size * _stp_nsubbufs;
diff -urNp a/runtime/transport/transport.c b/runtime/transport/transport.c
--- a/runtime/transport/transport.c	2013-05-21 15:51:54.000000000 +0400
+++ b/runtime/transport/transport.c	2013-05-21 16:45:47.638232907 +0400
@@ -20,6 +20,10 @@
 #include <linux/namei.h>
 #include <linux/delay.h>
 #include <linux/mutex.h>
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+#include <linux/cred.h>
+#include <linux/uidgid.h>
+#endif
 
 static int _stp_exit_flag = 0;
 
@@ -336,9 +340,14 @@ static int _stp_transport_init(void)
 	_stp_uid = current->uid;
 	_stp_gid = current->gid;
 #else
+#ifdef CONFIG_UIDGID_STRICT_TYPE_CHECKS
+	_stp_uid = from_kuid_munged(current_user_ns(), current_uid());
+	_stp_gid = from_kgid_munged(current_user_ns(), current_gid());
+#else
 	_stp_uid = current_uid();
 	_stp_gid = current_gid();
 #endif
+#endif
 
 /* PR13489, missing inode-uprobes symbol-export workaround */
 #if !defined(STAPCONF_TASK_USER_REGSET_VIEW_EXPORTED) && !defined(STAPCONF_UTRACE_REGSET) /* RHEL5 era utrace */

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: systemtap broken by USER_NS for now
  2013-05-22 14:00 systemtap broken by USER_NS for now Alexander Y. Fomichev
@ 2013-05-22 21:40 ` David Smith
  2013-05-31 19:49   ` David Smith
  0 siblings, 1 reply; 3+ messages in thread
From: David Smith @ 2013-05-22 21:40 UTC (permalink / raw)
  To: Alexander Y. Fomichev; +Cc: systemtap

On 05/22/2013 09:00 AM, Alexander Y. Fomichev wrote:
> Hi
> 
> for now user_ns unconditionally requires
> CONFIG_UIDGID_STRICT_TYPE_CHECKS and the latter in turn breaks things
> this way:
> 
> typedef struct {
>         uid_t val;
> } kuid_t;
> 
> and so:
> 
> stap # ./probe.stp
> In file included from /usr/share/systemtap/runtime/linux/task_finder.c:17:0,
>                  from /usr/share/systemtap/runtime/linux/runtime.h:188,
>                  from /usr/share/systemtap/runtime/runtime.h:24,
>                  from
> /tmp/.private/root/stap7M7WmA/stap_6ce999ddb8d32c70dcbe287153f6957f_16537_src.c:22:
> /usr/share/systemtap/runtime/linux/task_finder2.c: In function
> '__stp_utrace_attach_match_filename':
> /usr/share/systemtap/runtime/linux/task_finder2.c:816:11: error:
> incompatible types when assigning to type 'uid_t' from type 'kuid_t'
>   tsk_euid = task_euid(tsk);
>            ^
> .....................................................................
> 
> It seems like fix is straightforward - just use from_kuid_munged to
> get euid if CONFIG_UIDGID_STRICT_TYPE_CHECKS is defined. (really it
> always works on latest kernels but breaks older ones). I've attached a
> simple patch and it works for 3.9 (both with and without
> STRICT_TYPE_CHECKS) but i'm not systemtap guy so i could be completely
> wrong. Feel free to throw it out without reading :)
> 
> --
> Best regards.
>        Alexander Y. Fomichev <git.user@gmail.com>
> 

Hmm, thanks for finding this and developing the patch. The tricky part
here will be supporting 3.9 without breaking some of the earliest
kernels we support (like RHEL4's 2.6.9).

I'll look into your patch. I'm also wondering if some of the procfs
fixes I just put in will work with CONFIG_UIDGID_STRICT_TYPE_CHECKS.

I'll send out an update when I've figured something out.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: systemtap broken by USER_NS for now
  2013-05-22 21:40 ` David Smith
@ 2013-05-31 19:49   ` David Smith
  0 siblings, 0 replies; 3+ messages in thread
From: David Smith @ 2013-05-31 19:49 UTC (permalink / raw)
  To: Alexander Y. Fomichev; +Cc: systemtap

On 05/22/2013 04:40 PM, David Smith wrote:
> On 05/22/2013 09:00 AM, Alexander Y. Fomichev wrote:
>> Hi
>>
>> for now user_ns unconditionally requires
>> CONFIG_UIDGID_STRICT_TYPE_CHECKS and the latter in turn breaks things
>> this way:
>>
>> typedef struct {
>>         uid_t val;
>> } kuid_t;
>>
>> and so:
>>
>> stap # ./probe.stp
>> In file included from /usr/share/systemtap/runtime/linux/task_finder.c:17:0,
>>                  from /usr/share/systemtap/runtime/linux/runtime.h:188,
>>                  from /usr/share/systemtap/runtime/runtime.h:24,
>>                  from
>> /tmp/.private/root/stap7M7WmA/stap_6ce999ddb8d32c70dcbe287153f6957f_16537_src.c:22:
>> /usr/share/systemtap/runtime/linux/task_finder2.c: In function
>> '__stp_utrace_attach_match_filename':
>> /usr/share/systemtap/runtime/linux/task_finder2.c:816:11: error:
>> incompatible types when assigning to type 'uid_t' from type 'kuid_t'
>>   tsk_euid = task_euid(tsk);
>>            ^
>> .....................................................................
>>
>> It seems like fix is straightforward - just use from_kuid_munged to
>> get euid if CONFIG_UIDGID_STRICT_TYPE_CHECKS is defined. (really it
>> always works on latest kernels but breaks older ones). I've attached a
>> simple patch and it works for 3.9 (both with and without
>> STRICT_TYPE_CHECKS) but i'm not systemtap guy so i could be completely
>> wrong. Feel free to throw it out without reading :)
>>
>> --
>> Best regards.
>>        Alexander Y. Fomichev <git.user@gmail.com>
>>
> 
> Hmm, thanks for finding this and developing the patch. The tricky part
> here will be supporting 3.9 without breaking some of the earliest
> kernels we support (like RHEL4's 2.6.9).
> 
> I'll look into your patch. I'm also wondering if some of the procfs
> fixes I just put in will work with CONFIG_UIDGID_STRICT_TYPE_CHECKS.
> 
> I'll send out an update when I've figured something out.

I've checked your (a bit modified) work in as commit 8571631. As I
feared, I had to tweak it a bit to handle old kernels.

Thanks again.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-05-31 19:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-22 14:00 systemtap broken by USER_NS for now Alexander Y. Fomichev
2013-05-22 21:40 ` David Smith
2013-05-31 19:49   ` David Smith

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).