Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1250628 > unrolled thread

[RFC v3 8/9] kmsg: add predefined _PID, _TID, _COMM keywords to kmsg* log dict

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC v3 8/9] kmsg: add predefined _PID, _TID,  _COMM keywords to kmsg* log dict Paul Osmialowski <p.osmialowsk@samsung.com> - 2015-10-19 15:00 +0200
    Re: [RFC v3 8/9] kmsg: add predefined _PID, _TID, _COMM keywords to  kmsg* log dict kbuild test robot <lkp@intel.com> - 2015-10-19 16:20 +0200
    Re: [RFC v3 8/9] kmsg: add predefined _PID, _TID, _COMM keywords to  kmsg* log dict Andy Lutomirski <luto@amacapital.net> - 2015-10-19 19:50 +0200
      Re: [RFC v3 8/9] kmsg: add predefined _PID, _TID,  _COMM keywords to kmsg* log dict Paul Osmialowski <p.osmialowsk@samsung.com> - 2015-10-20 13:20 +0200
        Re: [RFC v3 8/9] kmsg: add predefined _PID, _TID, _COMM keywords to  kmsg* log dict Andy Lutomirski <luto@amacapital.net> - 2015-10-20 20:30 +0200

#1250628 — [RFC v3 8/9] kmsg: add predefined _PID, _TID, _COMM keywords to kmsg* log dict

From: Marcin Niesluchowski <m.niesluchow@samsung.com>

kmsg* devices write operation wrote no dict along with message
Due to usage of kmsg devices in userspace dict has been added
identifying pid, tid and comm of writing process.

Signed-off-by: Marcin Niesluchowski <m.niesluchow@samsung.com>
Signed-off-by: Paul Osmialowski <p.osmialowsk@samsung.com>
---
 kernel/printk/kmsg.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/kernel/printk/kmsg.c b/kernel/printk/kmsg.c
index 0f56fc9..8c904fe 100644
--- a/kernel/printk/kmsg.c
+++ b/kernel/printk/kmsg.c
@@ -28,6 +28,17 @@
 
 #define KMSG_MAX_MINOR_LEN	20
 
+#define MAX_PID_LEN		20
+#define MAX_TID_LEN		20
+/*
+ * Fromat below describes dict appended to message written from userspace:
+ * "_PID=<pid>\0_TID=<tid>\0_COMM=<comm>"
+ * KMSG_DICT_MAX_LEN definition represents maximal length of this dict.
+ */
+#define KMSG_DICT_MAX_LEN	(5 + MAX_PID_LEN + 1 + \
+				 5 + MAX_TID_LEN + 1 + \
+				 6 + TASK_COMM_LEN)
+
 /* /dev/kmsg - userspace message inject/listen interface */
 struct devkmsg_user {
 	u64 seq;
@@ -37,7 +48,23 @@ struct devkmsg_user {
 	char buf[CONSOLE_EXT_LOG_MAX];
 };
 
-static int kmsg_sys_write(int minor, int level, const char *fmt, ...)
+static size_t set_kmsg_dict(char *buf)
+{
+	size_t len;
+
+	len = sprintf(buf, "_PID=%d", task_tgid_nr(current)) + 1;
+	len += sprintf(buf + len, "_TID=%d", task_pid_nr(current)) + 1;
+	memcpy(buf + len, "_COMM=", 6);
+	len += 6;
+	get_task_comm(buf + len, current);
+	while (buf[len] != '\0')
+		len++;
+	return len;
+}
+
+static int kmsg_sys_write(int minor, int level,
+			  const char *dict, size_t dictlen,
+			  const char *fmt, ...)
 {
 	va_list args;
 	int ret = -ENXIO;
@@ -52,7 +79,7 @@ static int kmsg_sys_write(int minor, int level, const char *fmt, ...)
 
 		va_start(args, fmt);
 		log_format_and_store(log_b, 1 /* LOG_USER */, level,
-				     NULL, 0, fmt, args);
+				     dict, dictlen, fmt, args);
 		va_end(args);
 		wake_up_interruptible(&log_b->wait);
 
@@ -72,6 +99,8 @@ static ssize_t devkmsg_write(struct kiocb *iocb, struct iov_iter *from)
 	int level = default_message_loglevel;
 	int facility = 1;	/* LOG_USER */
 	size_t len = iov_iter_count(from);
+	char dict[KMSG_DICT_MAX_LEN];
+	size_t dictlen;
 	ssize_t ret = len;
 	int minor = iminor(iocb->ki_filp->f_inode);
 
@@ -111,10 +140,13 @@ static ssize_t devkmsg_write(struct kiocb *iocb, struct iov_iter *from)
 		}
 	}
 
+	dictlen = set_kmsg_dict(dict);
+
 	if (minor == log_buf.minor) {
-		printk_emit(facility, level, NULL, 0, "%s", line);
+		printk_emit(facility, level, dict, dictlen, "%s", line);
 	} else {
-		int error = kmsg_sys_write(minor, level, "%s", line);
+		int error = kmsg_sys_write(minor, level, dict, dictlen,
+					   "%s", line);
 
 		if (error)
 			ret = error;
-- 
1.9.1

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

[toc] | [next] | [standalone]

#1250741 — Re: [RFC v3 8/9] kmsg: add predefined _PID, _TID, _COMM keywords to kmsg* log dict

[Multipart message — attachments visible in raw view] — view raw

Hi Marcin,

[auto build test WARNING on next-20151016 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Paul-Osmialowski/Additional-kmsg-devices/20151019-211509
config: i386-defconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   kernel/printk/kmsg.c: In function 'set_kmsg_dict':
   kernel/printk/kmsg.c:55:32: error: implicit declaration of function 'task_tgid_nr' [-Werror=implicit-function-declaration]
     len = sprintf(buf, "_PID=%d", task_tgid_nr(current)) + 1;
                                   ^
   kernel/printk/kmsg.c:56:39: error: implicit declaration of function 'task_pid_nr' [-Werror=implicit-function-declaration]
     len += sprintf(buf + len, "_TID=%d", task_pid_nr(current)) + 1;
                                          ^
   kernel/printk/kmsg.c:59:2: error: implicit declaration of function 'get_task_comm' [-Werror=implicit-function-declaration]
     get_task_comm(buf + len, current);
     ^
   In file included from include/linux/fs.h:5:0,
                    from kernel/printk/kmsg.c:5:
   kernel/printk/kmsg.c: In function 'kmsg_sys_write':
   include/linux/wait.h:171:47: error: 'TASK_INTERRUPTIBLE' undeclared (first use in this function)
    #define wake_up_interruptible(x) __wake_up(x, TASK_INTERRUPTIBLE, 1, NULL)
                                                  ^
   kernel/printk/kmsg.c:84:3: note: in expansion of macro 'wake_up_interruptible'
      wake_up_interruptible(&log_b->wait);
      ^
   include/linux/wait.h:171:47: note: each undeclared identifier is reported only once for each function it appears in
    #define wake_up_interruptible(x) __wake_up(x, TASK_INTERRUPTIBLE, 1, NULL)
                                                  ^
   kernel/printk/kmsg.c:84:3: note: in expansion of macro 'wake_up_interruptible'
      wake_up_interruptible(&log_b->wait);
      ^
   kernel/printk/kmsg.c: In function 'devkmsg_write':
   kernel/printk/kmsg.c:40:10: error: 'TASK_COMM_LEN' undeclared (first use in this function)
         6 + TASK_COMM_LEN)
             ^
>> kernel/printk/kmsg.c:102:12: note: in expansion of macro 'KMSG_DICT_MAX_LEN'
     char dict[KMSG_DICT_MAX_LEN];
               ^
   kernel/printk/kmsg.c:102:7: warning: unused variable 'dict' [-Wunused-variable]
     char dict[KMSG_DICT_MAX_LEN];
          ^
   In file included from include/linux/fs.h:5:0,
                    from kernel/printk/kmsg.c:5:
   kernel/printk/kmsg.c: In function 'kmsg_read':
   include/linux/wait.h:400:31: error: 'TASK_INTERRUPTIBLE' undeclared (first use in this function)
     ___wait_event(wq, condition, TASK_INTERRUPTIBLE, 0, 0,  \
                                  ^
   include/linux/wait.h:225:52: note: in definition of macro '___wait_event'
      long __int = prepare_to_wait_event(&wq, &__wait, state);\
                                                       ^
   include/linux/wait.h:423:11: note: in expansion of macro '__wait_event_interruptible'
      __ret = __wait_event_interruptible(wq, condition); \
              ^
   kernel/printk/kmsg.c:181:10: note: in expansion of macro 'wait_event_interruptible'
       ret = wait_event_interruptible(log_b->wait,
             ^
   include/linux/wait.h:198:43: error: 'TASK_KILLABLE' undeclared (first use in this function)
      state == TASK_INTERRUPTIBLE || state == TASK_KILLABLE) \
                                              ^
   include/linux/wait.h:230:7: note: in expansion of macro '___wait_is_interruptible'
      if (___wait_is_interruptible(state) && __int) {  \
          ^
   include/linux/wait.h:400:2: note: in expansion of macro '___wait_event'
     ___wait_event(wq, condition, TASK_INTERRUPTIBLE, 0, 0,  \
     ^
   include/linux/wait.h:423:11: note: in expansion of macro '__wait_event_interruptible'
      __ret = __wait_event_interruptible(wq, condition); \
              ^
   kernel/printk/kmsg.c:181:10: note: in expansion of macro 'wait_event_interruptible'
       ret = wait_event_interruptible(log_b->wait,
             ^
   include/linux/wait.h:401:9: error: implicit declaration of function 'schedule' [-Werror=implicit-function-declaration]
            schedule())
            ^
   include/linux/wait.h:240:3: note: in definition of macro '___wait_event'
      cmd;       \
      ^
   include/linux/wait.h:423:11: note: in expansion of macro '__wait_event_interruptible'
      __ret = __wait_event_interruptible(wq, condition); \
              ^
   kernel/printk/kmsg.c:181:10: note: in expansion of macro 'wait_event_interruptible'
       ret = wait_event_interruptible(log_b->wait,
             ^
   kernel/printk/kmsg.c: In function 'kmsg_sys_buffer_del':
   include/linux/wait.h:171:47: error: 'TASK_INTERRUPTIBLE' undeclared (first use in this function)
    #define wake_up_interruptible(x) __wake_up(x, TASK_INTERRUPTIBLE, 1, NULL)
                                                  ^
   kernel/printk/kmsg.c:734:2: note: in expansion of macro 'wake_up_interruptible'
     wake_up_interruptible(&log_b->wait);
     ^
   cc1: some warnings being treated as errors

vim +/KMSG_DICT_MAX_LEN +102 kernel/printk/kmsg.c

    78			raw_spin_lock(&log_b->lock);
    79	
    80			va_start(args, fmt);
    81			log_format_and_store(log_b, 1 /* LOG_USER */, level,
    82					     dict, dictlen, fmt, args);
    83			va_end(args);
  > 84			wake_up_interruptible(&log_b->wait);
    85	
    86			raw_spin_unlock(&log_b->lock);
    87	
    88			ret = 0;
    89			break;
    90		}
    91		rcu_read_unlock();
    92		return ret;
    93	}
    94	
    95	static ssize_t devkmsg_write(struct kiocb *iocb, struct iov_iter *from)
    96	{
    97		char *buf, *line;
    98		int i;
    99		int level = default_message_loglevel;
   100		int facility = 1;	/* LOG_USER */
   101		size_t len = iov_iter_count(from);
 > 102		char dict[KMSG_DICT_MAX_LEN];
   103		size_t dictlen;
   104		ssize_t ret = len;
   105		int minor = iminor(iocb->ki_filp->f_inode);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1250963 — Re: [RFC v3 8/9] kmsg: add predefined _PID, _TID, _COMM keywords to kmsg* log dict

On Mon, Oct 19, 2015 at 5:58 AM, Paul Osmialowski
<p.osmialowsk@samsung.com> wrote:
> From: Marcin Niesluchowski <m.niesluchow@samsung.com>
>
> kmsg* devices write operation wrote no dict along with message
> Due to usage of kmsg devices in userspace dict has been added
> identifying pid, tid and comm of writing process.

Does this affect even the normal /dev/kmsg?

> -static int kmsg_sys_write(int minor, int level, const char *fmt, ...)
> +static size_t set_kmsg_dict(char *buf)
> +{
> +       size_t len;
> +
> +       len = sprintf(buf, "_PID=%d", task_tgid_nr(current)) + 1;
> +       len += sprintf(buf + len, "_TID=%d", task_pid_nr(current)) + 1;
> +       memcpy(buf + len, "_COMM=", 6);
> +       len += 6;
> +       get_task_comm(buf + len, current);
> +       while (buf[len] != '\0')
> +               len++;

len += strlen(buf); ?

Is it obvious for some reason that this doesn't overflow buf?

Why is task_pid_nr acceptable here?  Isn't this intended for use in namespaces?

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

[toc] | [prev] | [next] | [standalone]

#1251600

Hi Andy,

On Mon, 19 Oct 2015, Andy Lutomirski wrote:

> On Mon, Oct 19, 2015 at 5:58 AM, Paul Osmialowski
> <p.osmialowsk@samsung.com> wrote:
>> From: Marcin Niesluchowski <m.niesluchow@samsung.com>
>>
>> kmsg* devices write operation wrote no dict along with message
>> Due to usage of kmsg devices in userspace dict has been added
>> identifying pid, tid and comm of writing process.
>
> Does this affect even the normal /dev/kmsg?

Yes.

>
>> -static int kmsg_sys_write(int minor, int level, const char *fmt, ...)
>> +static size_t set_kmsg_dict(char *buf)
>> +{
>> +       size_t len;
>> +
>> +       len = sprintf(buf, "_PID=%d", task_tgid_nr(current)) + 1;
>> +       len += sprintf(buf + len, "_TID=%d", task_pid_nr(current)) + 1;
>> +       memcpy(buf + len, "_COMM=", 6);
>> +       len += 6;
>> +       get_task_comm(buf + len, current);
>> +       while (buf[len] != '\0')
>> +               len++;
>
> len += strlen(buf); ?
>
> Is it obvious for some reason that this doesn't overflow buf?
>

KMSG_DICT_MAX_LEN sets architecture-intepentent max size.

> Why is task_pid_nr acceptable here?  Isn't this intended for use in namespaces?
>

task_tgid_nr - process id (pid as seen in userspace),
task_pid_nr - thread id

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

[toc] | [prev] | [next] | [standalone]

#1251942 — Re: [RFC v3 8/9] kmsg: add predefined _PID, _TID, _COMM keywords to kmsg* log dict

On Tue, Oct 20, 2015 at 4:16 AM, Paul Osmialowski
<p.osmialowsk@samsung.com> wrote:
> Hi Andy,
>
> On Mon, 19 Oct 2015, Andy Lutomirski wrote:
>
>> On Mon, Oct 19, 2015 at 5:58 AM, Paul Osmialowski
>> <p.osmialowsk@samsung.com> wrote:
>>>
>>> From: Marcin Niesluchowski <m.niesluchow@samsung.com>
>>>
>>> kmsg* devices write operation wrote no dict along with message
>>> Due to usage of kmsg devices in userspace dict has been added
>>> identifying pid, tid and comm of writing process.
>>
>>
>> Does this affect even the normal /dev/kmsg?
>
>
> Yes.

Is this okay?  Comm at least is a bit odd.

>
>>
>>> -static int kmsg_sys_write(int minor, int level, const char *fmt, ...)
>>> +static size_t set_kmsg_dict(char *buf)
>>> +{
>>> +       size_t len;
>>> +
>>> +       len = sprintf(buf, "_PID=%d", task_tgid_nr(current)) + 1;
>>> +       len += sprintf(buf + len, "_TID=%d", task_pid_nr(current)) + 1;
>>> +       memcpy(buf + len, "_COMM=", 6);
>>> +       len += 6;
>>> +       get_task_comm(buf + len, current);
>>> +       while (buf[len] != '\0')
>>> +               len++;
>>
>>
>> len += strlen(buf); ?
>>
>> Is it obvious for some reason that this doesn't overflow buf?
>>
>
> KMSG_DICT_MAX_LEN sets architecture-intepentent max size.

And how many things are written and what asserts that it's big enough?

>
>> Why is task_pid_nr acceptable here?  Isn't this intended for use in
>> namespaces?
>>
>
> task_tgid_nr - process id (pid as seen in userspace),
> task_pid_nr - thread id

These are the ids as seen by the global namespace.  If the goal is to
create one of these nodes in some other namespace, then this is the
wrong thing to do.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web